Skip to content
This repository has been archived by the owner on Apr 6, 2021. It is now read-only.

RecordHandler has many thread safety issues #109

Closed
fhriley opened this issue Oct 9, 2017 · 0 comments
Closed

RecordHandler has many thread safety issues #109

fhriley opened this issue Oct 9, 2017 · 0 comments

Comments

@fhriley
Copy link
Contributor

fhriley commented Oct 9, 2017

The records, lists, and listeners HashMaps of RecordHandler are accessed and modified in non-thread safe ways. There is an attempt in some places to make it thread safe with the following double checked pattern:

Record record = records.get( name );
if( record == null ) {
    synchronized (this) {
        record = records.get( name );
        if (record == null) {
            record = new Record(name, new HashMap(), connection, deepstreamConfig, client);
            records.put(name, record);
...

But that allows HashMap.get to happen simultaneously with HashMap.put. The HashMap javadocs are very clear on this:

If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally.

yasserf added a commit that referenced this issue Nov 28, 2017
Fix thread safety problems. Fixes #109.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant