Odd ConcurrentModificationException

问题:

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);)

回答1:

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.