Thread Safety

While digging through the code to get answer to a question needed to proceed with a story authoring activity, I stumbled upon the following code. Most of the the class is omitted and the names of the routine/ structure simplified.

...

public void insertIntoMap(long key, Object value) {
    synchronized (readWriteLock) {
        this.map.put(key, value);
    }
}

...

public Map getMap() {
    synchronized (readWriteLock) {
        return this.map;
    }
}

...

The part that drew my attention was the getMap() method. Wait to acquire the lock, then return a reference to the object. The call of the method, when it access the object, is no longer protected by the readWriteLock. Therefore, we have the makings of a ConcurrentAccessException on our hands. There are two primary ways to resolve this issue.

Follow the copy on access pattern

public Map getMap() {
    synchronized (readWriteLock) {
        return this.map.clone();
    }
}

Eliminate the need for the read/ write lock

private static final ConcurrentHashMap<> map = new ConcurrentHashMap<Long, Object>();

...

public void insertIntoMap(long key, Object value) {
    this.map.put(key, value);
}

...

public Map getMap() {
    return this.map;
}

I'm a fan of the second approach. Using a data structure that has built in thread-safety reduces the likelihood the developer will forget to use a lock somewhere and introduce a concurrency problem.