[Reviewer: BPA] Add locking around access to a ManagedTable's row map

[rkday-pro]

The basic design is:

• every time we try to modify the map, we must take a lock - this ensures only one thread can do it at once
• we don't want to try to take the same lock twice - this will cause deadlock. For this reason, where we call other functions (e.g. 'get' calling 'add'), I've added a 'locked_add' function which you call if you;re already holding the lock. This is protected, so users of this class can't call it - only this class and its subclasses.

I was reluctant to add a lock here, as that might make call processing less efficient just to update some statistics - but I don't think we'll take the lock very often (only when we connect to or disconnect from a DGN) and obviously crashing is worse.

[mirw]

Don't we use ManagedTable for things like tracking numbers of different response types on Cx connections (via CxCounterTableImpl) - it's not just when we connect/disconnect a DGN? It seems like taking an additional lock on every access to that (and it looks to be every read, not just every add/remove) could be pretty expensive.

Not saying it's the wrong thing to do, but I think we to double-check the performance implications here.

This feels like quite an obvious issue - do you know how we expected this to work when we designed it originally?

[rkday-pro]

cpp-common/src/http_connection_pool.cpp

Line 112 in 3b35e9c

// The lock is held in the base class when this method is called, so it is

suggests we expected it to be protected by a lock, but it's not very clear.

I'll double-check how widely ManagedTable is used. In the interim, I'm going to ask @BennettAllen1 to finish and merge https://github.com/Metaswitch/cpp-common/pull/736/files as that does avoid the problem without a perf hit.