Fix boundary event iterator desynchronization in ContinueProcessOperation

[uucoding]

The executeBoundaryEvents method had a critical iterator desynchronization bug. The while loop assumed that the boundaryEvents and boundaryEventExecutions lists contained matching elements in the same order, but they did not due to inconsistent filtering.

For example:

• createBoundaryEvents returns only: [TimerExec, ErrorExec] (compensation events are skipped)
• executeBoundaryEvents receives: [Timer, Compensation, Error] (all events)

Loop pairing comparison:

Expected pairing:              Actual incorrect pairing:
Timer ↔ TimerExec ✓            Timer ↔ TimerExec ✓
Error ↔ ErrorExec ✓            Compensation ↔ ErrorExec ✗ 

[filiphr]

@uucoding can you please write a test case for this?

[uucoding]

@uucoding can you please write a test case for this?

@filiphr Sure, I've already submitted it. Please take a look.

[uucoding]

Hi @filiphr ,

Hope you're doing well!

Just circling back on the test case I submitted last month for the boundary event iterator desynchronization fix. When you have a moment, could you please review it and let me know if any changes are needed?

Thanks so much!

[filiphr]

Thanks for the test case @uucoding. I'll have a look at it, I still do not understand why this is happening since the boundaryEvents passed to the executeBoundaryEvents should pass createBoundaryEvents if we are not in a compensation.

[uucoding]

Hi @filiphr , thanks for looking into this!

The confusion comes from a subtle list-size mismatch caused by the continue statement inside the createBoundaryEvents method. It's quite easy to miss.

In the original code, if a BoundaryEvent is a CompensateEventDefinition, it hits the continue statement and skips creating an ExecutionEntity for it.

// Original code in createBoundaryEvents
for (BoundaryEvent boundaryEvent : boundaryEvents) {
    if (...) {
        if (... || (boundaryEvent.getEventDefinitions().get(0) instanceof CompensateEventDefinition)) {
            continue; // <--- THIS IS THE CULPRIT
        }
    }
    // Execution is created and added to boundaryEventExecutions
}

Because of this continue, the returned boundaryEventExecutions list can be shorter than the original boundaryEvents list. Then, in executeBoundaryEvents, the parallel iterators (while(events.hasNext() && executions.hasNext())) get misaligned, pairing the wrong event with the wrong execution.

[filiphr]

Gotcha, thanks @uucoding. Looking at that, shouldn't we then just change executeBoundaryEvents?

protected void executeBoundaryEvents(List<ExecutionEntity> boundaryEventExecutions) {
    if (!CollectionUtil.isEmpty(boundaryEventExecutions)) {

        for (ExecutionEntity boundaryEventExecution: boundaryEventExecutions) {
            BoundaryEvent boundaryEvent = (BoundaryEvent) boundaryEventExecution.getCurrentFlowElement();
            ActivityBehavior boundaryEventBehavior = ((ActivityBehavior) boundaryEvent.getBehavior());
            LOGGER.debug("Executing boundary event activityBehavior {} with execution {}", boundaryEventBehavior.getClass(), boundaryEventExecution.getId());
            boundaryEventBehavior.execute(boundaryEventExecution);
        }
    }
}

[uucoding]

Gotcha, thanks @uucoding. Looking at that, shouldn't we then just change executeBoundaryEvents?

protected void executeBoundaryEvents(List<ExecutionEntity> boundaryEventExecutions) {
    if (!CollectionUtil.isEmpty(boundaryEventExecutions)) {

        for (ExecutionEntity boundaryEventExecution: boundaryEventExecutions) {
            BoundaryEvent boundaryEvent = (BoundaryEvent) boundaryEventExecution.getCurrentFlowElement();
            ActivityBehavior boundaryEventBehavior = ((ActivityBehavior) boundaryEvent.getBehavior());
            LOGGER.debug("Executing boundary event activityBehavior {} with execution {}", boundaryEventBehavior.getClass(), boundaryEventExecution.getId());
            boundaryEventBehavior.execute(boundaryEventExecution);
        }
    }
}

Yes, absolutely! That is a much cleaner and more elegant solution.
Since the ExecutionEntity already holds the reference to the exact BoundaryEvent via getCurrentFlowElement(), passing the original boundaryEvents list is completely unnecessary. Your approach entirely removes the risky parallel iteration and simplifies the code significantly.
I will update my PR with this change shortly, including modifying the caller methods to drop the redundant parameter. Thanks for the brilliant suggestion!

[uucoding]

Hi @filiphr ,
I have just updated the PR based on your excellent suggestion.
The boundaryEvents parameter has been completely removed from the caller methods, and the execution logic now relies purely on execution.getCurrentFlowElement(). It is indeed much cleaner and safer now.
Thanks again for the guidance! Let me know if there is anything else needed for this to be merged.

[filiphr]

@uucoding I've manually merged this in 173fe6d. There was one more place that had the same problem, so I added that as well