fix: multi-instance collection expression is invoked with inconsistent delegate execution state

engine/src/main/java/org/camunda/bpm/engine/impl/bpmn/behavior/MultiInstanceActivityBehavior.java

@@ -73,26 +73,31 @@ else if (nrOfInstances < 0) {
     }
   }
 
-  protected void performInstance(ActivityExecution execution, PvmActivity activity, int loopCounter) {
+  protected void performInstance(ActivityExecution execution, PvmActivity activity, int loopCounter, Collection<?> collection) {
     setLoopVariable(execution, LOOP_COUNTER, loopCounter);
-    evaluateCollectionVariable(execution, loopCounter);
+    evaluateCollectionVariable(execution, collection, loopCounter);
     execution.setEnded(false);
     execution.setActive(true);
     execution.executeActivity(activity);
   }
 
-  protected void evaluateCollectionVariable(ActivityExecution execution, int loopCounter) {
+  protected void evaluateCollectionVariable(ActivityExecution execution, Collection<?> collection, int loopCounter) {
+    if (usesCollection() && collectionElementVariable != null && collection != null) {
+      Object value = getElementAtIndex(loopCounter, collection);
+      setLoopVariable(execution, collectionElementVariable, value);
+    }
+  }
+
+  protected Collection<?> evaluateCollection(ActivityExecution execution) {
+    Collection<?> collection = null;
     if (usesCollection() && collectionElementVariable != null) {
-      Collection<?> collection = null;
       if (collectionExpression != null) {
         collection = (Collection<?>) collectionExpression.getValue(execution);
       } else if (collectionVariable != null) {
         collection = (Collection<?>) execution.getVariable(collectionVariable);
       }
-
-      Object value = getElementAtIndex(loopCounter, collection);
-      setLoopVariable(execution, collectionElementVariable, value);
     }
+    return collection;
   }
 
   protected abstract void createInstances(ActivityExecution execution, int nrOfInstances) throws Exception;

engine/src/main/java/org/camunda/bpm/engine/impl/bpmn/behavior/ParallelMultiInstanceActivityBehavior.java

@@ -28,6 +28,7 @@
 import org.camunda.bpm.engine.impl.pvm.runtime.PvmExecutionImpl;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -40,6 +41,9 @@ public class ParallelMultiInstanceActivityBehavior extends MultiInstanceActivity
   protected void createInstances(ActivityExecution execution, int nrOfInstances) throws Exception {
     PvmActivity innerActivity = getInnerActivity(execution.getActivity());
 
+    // evaluate the collection to ensure the input of collectionExpression same as before
+    // also reduces the loop count of collectionExpression
+    Collection<?> collection = evaluateCollection(execution);
     // initialize the scope and create the desired number of child executions
     prepareScopeExecution(execution, nrOfInstances);
 
@@ -53,7 +57,7 @@ protected void createInstances(ActivityExecution execution, int nrOfInstances) t
     // actually be started in correct order :) )
     for (int i = (nrOfInstances - 1); i >= 0; i--) {
       ActivityExecution activityExecution = concurrentExecutions.get(i);
-      performInstance(activityExecution, innerActivity, i);
+      performInstance(activityExecution, innerActivity, i, collection);
     }
   }
 

engine/src/main/java/org/camunda/bpm/engine/impl/bpmn/behavior/SequentialMultiInstanceActivityBehavior.java

@@ -17,6 +17,7 @@
 package org.camunda.bpm.engine.impl.bpmn.behavior;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import org.camunda.bpm.engine.impl.ProcessEngineLogger;
@@ -35,27 +36,27 @@ public class SequentialMultiInstanceActivityBehavior extends MultiInstanceActivi
 
   @Override
   protected void createInstances(ActivityExecution execution, int nrOfInstances) throws Exception {
-
+    Collection<?> collection = evaluateCollection(execution);
     prepareScope(execution, nrOfInstances);
     setLoopVariable(execution, NUMBER_OF_ACTIVE_INSTANCES, 1);
 
     ActivityImpl innerActivity = getInnerActivity(execution.getActivity());
-    performInstance(execution, innerActivity, 0);
+    performInstance(execution, innerActivity, 0, collection);
   }
 
   public void complete(ActivityExecution scopeExecution) {
     int loopCounter = getLoopVariable(scopeExecution, LOOP_COUNTER) + 1;
     int nrOfInstances = getLoopVariable(scopeExecution, NUMBER_OF_INSTANCES);
     int nrOfCompletedInstances = getLoopVariable(scopeExecution, NUMBER_OF_COMPLETED_INSTANCES) + 1;
-
     setLoopVariable(scopeExecution, NUMBER_OF_COMPLETED_INSTANCES, nrOfCompletedInstances);
 
     if (loopCounter == nrOfInstances || completionConditionSatisfied(scopeExecution)) {
       leave(scopeExecution);
     }
     else {
+      Collection<?> collection = evaluateCollection(scopeExecution);
       PvmActivity innerActivity = getInnerActivity(scopeExecution.getActivity());
-      performInstance(scopeExecution, innerActivity, loopCounter);
+      performInstance(scopeExecution, innerActivity, loopCounter, collection);
     }
   }
 

engine/src/test/java/org/camunda/bpm/engine/test/bpmn/multiinstance/MultiInstanceTest.java

@@ -55,7 +55,8 @@
 import org.camunda.bpm.engine.test.util.PluggableProcessEngineTest;
 import org.camunda.bpm.engine.variable.VariableMap;
 import org.camunda.bpm.engine.variable.Variables;
-import org.junit.Ignore;
+import org.camunda.bpm.model.bpmn.Bpmn;
+import org.camunda.bpm.model.bpmn.BpmnModelInstance;
 import org.junit.Test;
 
 
@@ -422,7 +423,6 @@ public void testEmptyCollectionInMI() {
   }
 
   @Deployment
-  @Ignore
   @Test
   public void testParallelUserTasksBasedOnCollectionExpression() {
     DelegateEvent.clearEvents();
@@ -439,6 +439,37 @@ public void testParallelUserTasksBasedOnCollectionExpression() {
     DelegateEvent.clearEvents();
   }
 
+  @Test
+  public void testSequentialUserTasksBasedOnCollectionExpression() {
+    DelegateEvent.clearEvents();
+
+    BpmnModelInstance model = Bpmn.createExecutableProcess("process")
+        .startEvent()
+        .userTask("miTasks")
+            .multiInstance()
+            .sequential()
+            .camundaCollection("${myBean.resolveCollection(execution)}")
+            .camundaElementVariable("elementVar")
+            .multiInstanceDone()
+        .endEvent()
+        .done();
+
+    testRule.deploy(model);
+
+    runtimeService.startProcessInstanceByKey("process",
+        Variables.createVariables().putValue("myBean", new DelegateBean()));
+
+    Task singleResult = taskService.createTaskQuery().singleResult();
+    taskService.complete(singleResult.getId());
+    List<DelegateEvent> recordedEvents = DelegateEvent.getEvents();
+    assertEquals(2, recordedEvents.size());
+
+    assertEquals("miTasks#multiInstanceBody", recordedEvents.get(0).getCurrentActivityId());
+    assertEquals("miTasks#multiInstanceBody", recordedEvents.get(1).getCurrentActivityId()); // or miTasks
+
+    DelegateEvent.clearEvents();
+  }
+
   @Deployment
   @Test
   public void testParallelUserTasksCustomExtensions() {