Adding Event Listener not Thread Safe

[elmarweber]

Hello,

when adding a event listener during runtime I get (in unit tests) a ConcurrentModificationException because at the same time I add the listener an event is dispatched (see exception below).

The error occurs here, while dispatching an event through the listeners the eventListeners array is modified.

[java]
         for (ActivitiEventListener listener : eventListeners) {
            dispatchEvent(event, listener);
         }
[/java]

Is this a bug or intended behavior, e.g. it is not intended to add an event listener during operations?

Activiti 5.16.4

Thank you,
Elmar

[error]    ConcurrentModificationException:   (ActivitiEventSupport.java:89)[error] org.activiti.engine.delegate.event.impl.ActivitiEventSupport.dispatchEvent(ActivitiEventSupport.java:89)[error] org.activiti.engine.delegate.event.impl.ActivitiEventDispatcherImpl.dispatchEvent(ActivitiEventDispatcherImpl.java:65)[error] org.activiti.engine.impl.pvm.runtime.AtomicOperationTransitionNotifyListenerTake.execute(AtomicOperationTransitionNotifyListenerTake.java:74)[error] org.activiti.engine.impl.interceptor.CommandContext.performOperation(CommandContext.java:96)[error] org.activiti.engine.impl.persistence.entity.ExecutionEntity.performOperationSync(ExecutionEntity.java:621)[error] org.activiti.engine.impl.persistence.entity.ExecutionEntity.performOperation(ExecutionEntity.java:616)[error] org.activiti.engine.impl.pvm.runtime.AtomicOperationTransitionDestroyScope.execute(AtomicOperationTransitionDestroyScope.java:116)[error] org.activiti.engine.impl.interceptor.CommandContext.performOperation(CommandContext.java:96)[error] org.activiti.engine.impl.persistence.entity.ExecutionEntity.performOperationSync(ExecutionEntity.java:621)[error] org.activiti.engine.impl.persistence.entity.ExecutionEntity.performOperation(ExecutionEntity.java:616)‍‍‍‍‍‍‍‍‍‍‍‍‍

[jbarrez]

No, looking at the code it was clearly not designed to add an event listener during the dispatching of another event.

What's the use case for this?

[elmarweber]

We are adding a listener for certain executions of single instances and them removing them. Overall more complicated topic, in the end we use Activiti to generate execution correct flows for simulation on large datasets (can elaborate on that when someone is really interested =).

At the moment we worked around it by registering a global listener and then registering our temporary listener to it. Our global listener is thread safe and makes sure this does not happen anymore (will be replaced with an Akka actor system in the future).

IMHO this is behavior that should be fixed in Activiti, as it can also happen during normal operations as well. E.g. when using a server wide Activiti engine that has several applications deployed using it.

Let me know your thoughts.

[jbarrez]

I can see vaguely the use cas,e but not yet fully convinced. I'm always very weary of adding synchronized code, as it slows things down obviously.

Or is it only the collection of event listeners you want to make thread safe? Ie a simple synchronizedMap would suffice?

[elmarweber]

Yes, only the registration needs synchronization, so the collection is fine.

[jbarrez]

Okay. Can you create a pull request for it so we can discuss it further there?

[faizal-manan]

any update regarding this issue (<a href="https://github.com/Activiti/Activiti/issues/886">Issue 886</a>)

[jbarrez]

One option to solve this is to remove the event listener after they're fired, for example using a CommandContextCloseListener.

[elmarweber]

Just an update from me, unfortunately I can't find the code anymore and I'm trying to remember how we fixed it, I think we synchronized externally as a fast work around. In the end we switched to a simpler BPMN engine implementation that is more light weight because we were just generating execution flows for simulation purposes.

[jbarrez]

FYI: The event list was changed to a CopyOnWrite implementation this week, so this problem should be solved.