TOMEE-4699 Fix ConcurrentModificationException in ThreadContext constructor

[JorrenH]

See Jira ticket for full context.

In summary:

• HashMap#putAll on a synchronizedMap is not thread-safe. It will access .entrySet().iterator() internally, allowing it to cause a ConcurrentModificationException.
• This PR simply synchronizes on the data so the iterator call remains safe.

Note: ConcurrentHashMap does not work here because it does not allow null values.

[otbutz]

• HashMap#putAll on a synchronizedMap is not thread-safe. It will access .entrySet().iterator() internally, allowing it to cause a ConcurrentModificationException.

To be precise, there are two mutexes involved: putAll only synchronises on this.data. However, accessing that.data via an iterator happens without synchronisation, which violates the contract of Collections.synchronizedMap().

[otbutz]

I see no reason to copy the whole map?

[rzo1]

LGTM, thanks for tracking this down.

I pushed a reproducer to the branch: ThreadContextCopyRaceTest copies a ThreadContext on one thread while another updates it. It fails within about half a second without the synchronized block and passes with it, so it should be a lot easier to work with than reproducing under load with a debugger attached. Feel free to change or drop it if you would rather keep the PR minimal.

I left one non-blocking note on the dataToString thread about toString() reading the size outside the lock.

[JorrenH]

@rzo1 Good to have a quick reproducer, thanks! I updated toString() such that the size is consistent with the entries since it's a quick win. Did pull the whole line into dataToString to avoid a change in the format (data=(2)=.. vs data(2)=..)