1

I am testing an event system I am writing for a project. In said project and tests, I do not touch threads. Literally, I do not create a thread or do anything with threads. However, I am getting a ConcurrentModificationException.

I understand that there are other situations in which this exception may be thrown. From the CME JavaDoc:

Note that this exception does not always indicate that an object has been concurrently modified by a different thread. If a single thread issues a sequence of method invocations that violates the contract of an object, the object may throw this exception.

Here is my test code:

TestListener test = new TestListener();
Assert.assertTrue(evtMgr.register(test));
Assert.assertFalse(testBool);
evtMgr.fire(new QuestStartEvent(null));
Assert.assertTrue(testBool);

testBool = false;
evtMgr.unregister(test);
evtMgr.fire(new QuestStartEvent(null));
Assert.assertFalse(testBool);

EventManager looks like this:

public boolean register(Listener listener) {
    return listeners.add(new ListenerHandle(listener));
}

public void unregister(Listener listener) {
    listeners.stream().filter((l) -> l.getListener() == listener)
            .forEach(listeners::remove);
}

public <T extends Event> T fire(T event) {
    listeners.forEach((listener) -> listener.handle(event));
    return event;
}

Where ConcurrentModificationException is at .forEach(listeners::remove);

ListenerHandle looks like this:

private final Listener listener;
private final Map<Class<? extends Event>, Set<MethodHandle>> eventHandlers;

public ListenerHandle(Listener listener) {
    this.listener = listener;
    this.eventHandlers = new HashMap<>();

    for (Method meth : listener.getClass().getDeclaredMethods()) {
        EventHandler eh = meth.getAnnotation(EventHandler.class);
        if (eh == null || meth.getParameterCount() != 1) {
            continue;
        }

        Class<?> parameter = meth.getParameterTypes()[0];
        if (!Event.class.isAssignableFrom(parameter)) {
            continue;
        }

        Class<? extends Event> evtClass = parameter.asSubclass(Event.class);
        MethodHandle handle = MethodHandles.lookup().unreflect(meth);
        Set<MethodHandle> handlers = eventHandlers.get(evtClass);
        if (handlers == null) {
            handlers = new HashSet<>();
            eventHandlers.put(evtClass, handlers);
        }

        handlers.add(handle);
    }
}

public void handle(Event event) {
    Class<? extends Event> clazz = event.getClass();
    Set<MethodHandle> handles = eventHandlers.get(clazz);
    if (handles == null || handles.isEmpty()) {
        return;
    }

    for (MethodHandle handle : handles) {
        handle.invoke(listener, event);
    }
}

(With try-catches cut for readability)

And the stack trace:

java.util.ConcurrentModificationException
    at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1545)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:512)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:502)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
    at EventManager.unregister(EventManager.java:54)

(Line 54 is .forEach(listeners::remove);)

OllieStanley
  • 686
  • 5
  • 22
  • 2
    You need remove by iterator, not List. – xio4 Sep 14 '14 at 11:41
  • @xio4 thanks, do you have an explanation as to why? I'd like to know more about the problem so I don't run into it again in the future – OllieStanley Sep 14 '14 at 11:44
  • 1
    http://stackoverflow.com/questions/1196586/calling-remove-in-foreach-loop-in-java – xio4 Sep 14 '14 at 11:45
  • @xio4 ah of course, makes sense - thanks for the help – OllieStanley Sep 14 '14 at 11:47
  • 1
    Possible duplicate of [Iterating through a Collection, avoiding ConcurrentModificationException when removing in loop](http://stackoverflow.com/questions/223918/iterating-through-a-collection-avoiding-concurrentmodificationexception-when-re) – Raedwald Mar 28 '16 at 14:47

1 Answers1

0

You're getting a concurrent modification exception because you're modifying the collection at the same time as iterating over it. Instead of

listeners.stream().filter((l) -> l.getListener() == listener)
         .forEach(listeners::remove);

you should either use a traditional Iterator based idiom and call remove() on the iterator rather than the collection, or first iterate to gather the set of objects you want to remove and then remove them in one go once the initial iteration is finished:

listeners.removeAll(
   listeners.stream().filter((l) -> l.getListener() == listener)
            .collect(Collectors.toList()));

The iterator approach is probably more efficient.

Ian Roberts
  • 117,925
  • 15
  • 163
  • 179