CODEWITHSUNDEEP
javamultithreadingevent-handling
lannyf
lannyf

Reputation: 11035

how to avoid ConcurrentModificationException for listen once listener case

Having a mEventMap to hold the listeners for different events, and has addListener() to register the eventlistener, removeListener(), and dispatchEvent() to the registered listeners.

public void addListener(EventListener listener) {
    synchronized (mEventMap) {

        List<WeakReference<EventListener<Event>>> listeners = mEventMap.get(listener.mEventClass);
WeakReference<EventListener<Event>> listenerRef = new WeakReference<>(
                (EventListener<Event>) listener)
        …
        listeners.add(listenerRef);

        …
    }

}

public void removeListener(EventListener listener) {
    synchronized (mEventMap) {
        List<WeakReference<EventListener<Event>>> listeners = mEventMap.get(listener.mEventClass);

        …
        if (contains(listeners, listener)) {
                doRemove(listeners, listener);
            }
       …

    }
}


public boolean dispatchEvent(Event event) {
synchronized (mEventMap) {
    List<WeakReference<EventListener<Event>>> listeners = mEventMap.get(event.getClass());
    ListIterator<WeakReference<EventListener<Event>>> listenerIterator = listeners.listIterator(listeners.size());
    …
    while (listenerIterator.hasPrevious()) {
            WeakReference<EventListener<Event>> listenerItem = listenerIterator.previous();
            EventListener<Event> listenerRef = listenerItem.get();
            if (listenerRef != null) {
                listenerRef.onEvent(event);
            } else {
                listenerIterator.remove();
            }
        }

    …
}

use case

EventListener<Event> mEventListener = new EventListener<Event>(
        Event.class) {
    @Override
    public boolean onEvent(Event event) {
        eMgr.removeListener(mEventListener);
        // do something
    }
};

addEventListener(mEventListener);

in dispatchEvent(), while it is in the loop the removeListener() is called and causes the ConcurrentModificationException at listenerItem = listenerIterator.previous();

question: what is the best way to avoid the crash caused by the change of the mEventMap data while someone is iterating on it.

Upvotes: 0

Views: 1496

Answers (4)

Jeff J
Jeff J

Reputation: 26

Another approach would be to use java's built in EventListenerList class https://docs.oracle.com/javase/8/docs/api/index.html?javax/swing/event/EventListenerList.html. Although it is in the javax.swing packages, I've found it very useful for event notification in general. It is backed by an array, manages listeners by class, and does not throw concurrent mod exceptions. Firing an event is done like so:

Object[] listeners = listenerList.getListenerList();
    for(int i = listeners.length - 2; i >= 0; i -= 2){
        if(listeners[i] == YourListenerClass.class){
            ((YourListenerClass)listeners[i + 1]).yourListenerMethod(yourEvent);
        }

    }

Upvotes: 0

lannyf
lannyf

Reputation: 11035

The approach to choose is to add a flag in the listener object, 'isRemoved'. So the removeListener would only mark this listener has been moved, but it will stay in the list until the dispatch time which will only dispatch to the ones not marked. And all marked ones will be removed from list after the dispatch loop.

Upvotes: 0

mambax
mambax

Reputation: 1117

Your problem is that you want to remove an element from the list while traversing it with an implicit iterator.

You could solve that by using explicitly an iterator:

for (Iterator<EventListener> it = list.iterator(); it.hasNext(); ) {
    EventListener el = it.next();
    it.remove();
}

Upvotes: 2

Nicolas Filotto
Nicolas Filotto

Reputation: 44995

Create a safe copy of your list in dispatchEvent before the loop something like:

List<WeakReference<EventListener<Event>>> listeners 
                          = new ArrayList<>(mEventMap.get(event.getClass()));

But by far the best approach for listeners is to use CopyOnWriteArrayList to manage the list of listeners as you don't modify it too often and it is already thread safe such that you don't need any synchronized blocks anymore.

Upvotes: 1

Related Questions