Concurrent access to HashMap

[ikoblik]

In AbstractSyncMessageBus field subscriptionsPerListener is accessed asynchronously. Simplest solution would be to use ReadWriteLock or ConcurrentHashMap. Please see this discussion:

http://www.coderanch.com/t/232685/threads/java/Concurrent-access-HashMap

[ikoblik]

Same issue in AbstractConcurrentSet.

[bennidi]

Yes, it is accessed concurrently without synchronization...but read-only. And that's the point - you don't need to synchronize read access to any data structure. I designed the access to all data structures according to this principle to increase through-put. The thread you are pointing to actually talks about concurrent read/write access, which of course needs to be synchronized.

[ikoblik]

you don't need to synchronize read access to any data structure

That's a little too broad, it depends on implementation of the data structure in question.

Please see the JavaDoc of HashMap: http://docs.oracle.com/javase/6/docs/api/java/util/HashMap.html . Line starting with "Note that this implementation is not synchronized.".

There's blog post stating that non-synchronized access can lead to an infinite loop:
http://lightbody.net/blog/?p=307

[ikoblik]

Hi bennidi,

I had a little spare time today to write unit test showing the issue. It's quite hard to reproduce infinite cycle but it's quite easy to show inconsistency arising from concurrent update while reading HashMap. You can find the unit test here:
https://gist.github.com/ikoblik/5545835

Hope it helps!
Ivan.

[bennidi]

Interesting! Thanks for pointing that out. I haven't thought about the contains() method as it is not a vital part of my code currently. But it definitely needs to be in sync with the writes. I think I will introduce a ReadWriteLock now. As for the subscription code. The rehashing issue also exists in that part of the code then. I will take a closer look and fix that asap.

you don't need to synchronize read access to any data structure

You're right, in the end it depends on the implementation -> repeated reads to the same elements/segments/whatever might lead to internal optimizations of the data structure that result in reordering and thus, writes. But as for the HashMap and article you pointed to: It clearly says, that at least one thread needs to modify the map to make it behave unpredictably. So usually, concurrent reads don't need to be synchronized.

Anyways, thanks a lot for taking the time to provide that test case and shed some light on this specific detail of maps rehashing behavior.

[ikoblik]

Sure, glad to help! Thank you for the good work you're doing.

[bennidi]

I adapted your test case and reproduced the error you described. It includes your documentation and I mentioned your name and blog URL to give you some credit - if that's ok with you.

I introduced the ReentrantReadWriteLock to correctly synchronize the set implementation. I will do the same for the subscription code. It will definitely make it into the next release which should be ready within a week or so.

[ikoblik]

Hi! You didn't have to link to me. The linked test is published under MIT or any other license you chose to use for mbassador in future. It was pleasure interacting with you!