From d5d3eb0cb4549801f10371cb2496677786cd55ff Mon Sep 17 00:00:00 2001
From: Remco Westerhoud <remco@westerhoud.nl>
Date: Tue, 15 Nov 2022 16:18:22 +0100
Subject: [PATCH] fix(engine): prevent NPE when terminating call activity

If a parent process has an active call activity element instance there is no guarantee that the called activity is still activate. When the called activity is completed it will try to complete the call activity element. At this point we try to apply the output mappings. If something goes wrong here an incident is created.
When we try to terminate the call activity at this point the called activity is already completed and no instance of it will be available. For this reason we must verify that this instance is not null before we try to terminate it.

(cherry picked from commit 254a863703d540b365e36ca63ac4995d2bd2d343)
---
 .../ProcessInstanceModificationProcessor.java |  4 +-
 .../ModifyProcessInstanceTest.java            | 49 +++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/engine/src/main/java/io/camunda/zeebe/engine/processing/processinstance/ProcessInstanceModificationProcessor.java b/engine/src/main/java/io/camunda/zeebe/engine/processing/processinstance/ProcessInstanceModificationProcessor.java
index 9f41b67c4aa6..9e81cc0399fb 100644
--- a/engine/src/main/java/io/camunda/zeebe/engine/processing/processinstance/ProcessInstanceModificationProcessor.java
+++ b/engine/src/main/java/io/camunda/zeebe/engine/processing/processinstance/ProcessInstanceModificationProcessor.java
@@ -572,7 +572,9 @@ private void terminateElement(
     } else if (elementType == BpmnElementType.CALL_ACTIVITY) {
       final var calledActivityElementInstance =
           elementInstanceState.getInstance(elementInstance.getCalledChildInstanceKey());
-      terminateElement(calledActivityElementInstance, sideEffects);
+      if (calledActivityElementInstance != null && calledActivityElementInstance.canTerminate()) {
+        terminateElement(calledActivityElementInstance, sideEffects);
+      }
     }
 
     stateWriter.appendFollowUpEvent(
diff --git a/engine/src/test/java/io/camunda/zeebe/engine/processing/processinstance/ModifyProcessInstanceTest.java b/engine/src/test/java/io/camunda/zeebe/engine/processing/processinstance/ModifyProcessInstanceTest.java
index f89e24e959e5..81c394c7e314 100644
--- a/engine/src/test/java/io/camunda/zeebe/engine/processing/processinstance/ModifyProcessInstanceTest.java
+++ b/engine/src/test/java/io/camunda/zeebe/engine/processing/processinstance/ModifyProcessInstanceTest.java
@@ -17,6 +17,7 @@
 import io.camunda.zeebe.model.bpmn.builder.SubProcessBuilder;
 import io.camunda.zeebe.protocol.record.Record;
 import io.camunda.zeebe.protocol.record.RecordType;
+import io.camunda.zeebe.protocol.record.intent.IncidentIntent;
 import io.camunda.zeebe.protocol.record.intent.JobIntent;
 import io.camunda.zeebe.protocol.record.intent.ProcessInstanceIntent;
 import io.camunda.zeebe.protocol.record.intent.ProcessInstanceModificationIntent;
@@ -850,6 +851,54 @@ public void shouldActivateParallelGateway() {
         .hasSize(4);
   }
 
+  @Test
+  public void verifyCallActivityWithIncidentInOutputMappingCanBeTerminated() {
+    final var child = Bpmn.createExecutableProcess("child").startEvent().endEvent().done();
+    final var parent =
+        Bpmn.createExecutableProcess(PROCESS_ID)
+            .startEvent()
+            .callActivity("callActivity", c -> c.zeebeProcessId("child"))
+            .zeebeOutputExpression("x", "y")
+            .manualTask("task")
+            .endEvent()
+            .done();
+
+    ENGINE.deployment().withXmlResource(child).withXmlResource(parent).deploy();
+
+    final var processInstanceKey = ENGINE.processInstance().ofBpmnProcessId(PROCESS_ID).create();
+
+    final var callActivityElement =
+        RecordingExporter.processInstanceRecords(ProcessInstanceIntent.ELEMENT_ACTIVATED)
+            .withProcessInstanceKey(processInstanceKey)
+            .withElementId("callActivity")
+            .withElementType(BpmnElementType.CALL_ACTIVITY)
+            .getFirst();
+
+    Assertions.assertThat(
+            RecordingExporter.incidentRecords(IncidentIntent.CREATED)
+                .withProcessInstanceKey(processInstanceKey)
+                .getFirst())
+        .extracting(r -> r.getValue().getElementId())
+        .isEqualTo("callActivity");
+
+    ENGINE
+        .processInstance()
+        .withInstanceKey(processInstanceKey)
+        .modification()
+        .activateElement("task")
+        .terminateElement(callActivityElement.getKey())
+        .modify();
+
+    verifyThatRootElementIsActivated(processInstanceKey, "task", BpmnElementType.MANUAL_TASK);
+    verifyThatProcessInstanceIsCompleted(processInstanceKey);
+    Assertions.assertThat(
+            RecordingExporter.processInstanceRecords(ProcessInstanceIntent.ELEMENT_TERMINATED)
+                .withProcessInstanceKey(processInstanceKey)
+                .withElementId(callActivityElement.getValue().getElementId())
+                .exists())
+        .isTrue();
+  }
+
   private static void verifyThatRootElementIsActivated(
       final long processInstanceKey, final String elementId, final BpmnElementType elementType) {
     verifyThatElementIsActivated(processInstanceKey, elementId, elementType, processInstanceKey);