CODEWITHSUNDEEP
javaconcurrency
zapl
zapl

Reputation: 63955

Listener / Observable implementation, synchronized vs concurrent collections

When implementing thread-safe listeners I usually wonder which type of Collection is the best to hold the listeners. I've found three options so far.

The standard Observable uses synchronized access to a simple ArrayList. Using a copy of the listeners is optional but as far as I can tell a good idea since it prevents problems like

Implementation is unfortunately more than a few lines. Collections.synchronizedList() would not require synchronized in removeListener but that's not really worth it.

class ObservableList {
    private final List<Listener> listeners = new ArrayList<Listener>();
    public void addListener(Listener listener) {
        synchronized (listeners) {
            if (!listeners.contains(listener)) {
                listeners.add(listener);
            }
        }
    }
    public void removeListener(Listener listener) {
        synchronized (listeners) {
            listeners.remove(listener);
        }
    }
    protected void notifyChange() {
        Listener[] copyOfListeners;
        synchronized (listeners) {
            copyOfListeners = listeners.toArray(new Listener[listeners.size()]);
        }
        // notify w/o synchronization
        for (Listener listener : copyOfListeners) {
            listener.onChange();
        }
    }
}

But there are Collections in java.util.concurrent that are inherently thread-safe and potentially more efficient since I would assume that their internal locking mechanism is optimized better than a simple synchronized block. Creating a copy for every notification is also quite expensive.

Based on CopyOnWriteArrayList

class ObservableCopyOnWrite {
    private final CopyOnWriteArrayList<Listener> listeners = new CopyOnWriteArrayList<>();
    public void addListener(Listener listener) {
        listeners.addIfAbsent(listener);
    }

    public void removeListener(Listener listener) {
        listeners.remove(listener);
    }

    protected void notifyChange() {
        for (Listener listener : listeners) {
            listener.onChange();
        }
    }
}

Should do roughly what the first version does but with less copies. Adding / removing listeners is not a very frequent action which means that copies should not be very frequent as well.

The version I usually use is based on ConcurrentHashMap which states for .keySet() which is used as Set here:

The view's iterator is a "weakly consistent" iterator that will never throw ConcurrentModificationException, and guarantees to traverse elements as they existed upon construction of the iterator, and may (but is not guaranteed to) reflect any modifications subsequent to construction.

Which means that iteration includes at least every listener that was registered when starting the iteration and may even include new ones that were added during iteration. Not sure about listeners that were removed. I do like this version because it's not copying and as simple to implement as CopyOnWriteArrayList.

class ObservableConcurrentSet {
    private final Set<Listener> listeners = Collections.newSetFromMap(new ConcurrentHashMap<Listener, Boolean>());

    public void addListener(Listener listener) {
        listeners.add(listener);
    }

    public void removeListener(Listener listener) {
        listeners.remove(listener);
    }

    protected void notifyChange() {
        for (Listener listener : listeners) {
            listener.onChange();
        }
    }
}

Is a ConcurrentHashMap based implementation a good idea or did I overlook something here? Or is there an even better Collection to use?

Upvotes: 4

Views: 3227

Answers (2)

Gray
Gray

Reputation: 116858

Is a ConcurrentHashMap based implementation a good idea or did I overlook something here? Or is there an even better Collection to use?

ConcurrentHashMap seems to be to be fine in this case. It certainly is better than using Collections.synchronizedList(). The CHM iterator may or may not see new entries in the map if they are added while the iterator is walking the map. It also may or may not see old entries if they are deleted during iteration. Both of these are due to race conditions as to when the nodes in question were added/removed and where the iterator is positioned. But the iterator will always see items that were in the map when it began as long as they were not deleted.

Upvotes: 2

Holger
Holger

Reputation: 298123

There is a pattern that outperforms all of these variants since Java 1.1. Look at the AWTEventMulticaster class and how it works. It offers thread-safety through immutability and has therefore no additional overhead. It even provides the most efficient way to handle the case when there is no or exactly one listener. Well, I think it offers the most efficient event delivery in general.

See http://docs.oracle.com/javase/7/docs/api/java/awt/AWTEventMulticaster.html

Look into the source code of this class if you want to know how to implement such pattern for own event types.

It is a pity that the Swing developer did not know about this and created that awful EventListenerList leading developers on a wrong path.

And by the way this pattern also solves the problem that listeners added during event delivery should not see the currently delivered event(s) which occurred before the addition. For free. Since Java 1.1

Upvotes: 2

Related Questions