NPE terminating both multi-instance body and child elements during PI modification

[remcowesterhoud]

Describe the bug

Sending a modification request in which we terminate both the multi-instance body as well as any of the elements inside of this multi-instance body will result in a NPE.

Terminating the multi-instance body, as well as the subprocess will trigger the NPE.

To Reproduce

Send a terminate request for a multi-instance and any of the element instances within.

Unit test

  @Test
  public void test() {
    // given
    ENGINE
        .deployment()
        .withXmlResource(
            Bpmn.createExecutableProcess(PROCESS_ID)
                .startEvent()
                .subProcess(
                    "A",
                    s ->
                        s.embeddedSubProcess()
                            .startEvent()
                            .userTask("B")
                            .endEvent()
                            .subProcessDone()
                            .multiInstance()
                            .parallel()
                            .zeebeInputCollectionExpression("[1,2,3]")
                            .multiInstanceDone())
                .endEvent()
                .done())
        .deploy();
    final var processInstanceKey = ENGINE.processInstance().ofBpmnProcessId(PROCESS_ID).create();

    final var multiInstanceElement =
        RecordingExporter.processInstanceRecords(ProcessInstanceIntent.ELEMENT_ACTIVATED)
            .withProcessInstanceKey(processInstanceKey)
            .withElementId("A")
            .withElementType(BpmnElementType.MULTI_INSTANCE_BODY)
            .getFirst();
    RecordingExporter.processInstanceRecords(ProcessInstanceIntent.ELEMENT_ACTIVATED)
        .withProcessInstanceKey(processInstanceKey)
        .withElementId("B")
        .withElementType(BpmnElementType.USER_TASK)
        .limit(3)
        .map(Record::getKey)
        .toList();
    final var elements =
        RecordingExporter.processInstanceRecords(ProcessInstanceIntent.ELEMENT_ACTIVATED)
            .withProcessInstanceKey(processInstanceKey)
            .withElementType(BpmnElementType.SUB_PROCESS)
            .limit(3)
            .map(Record::getKey)
            .toList();

    // when
    ENGINE
        .processInstance()
        .withInstanceKey(processInstanceKey)
        .modification()
        .terminateElement(multiInstanceElement.getKey())
        .terminateElement(elements.get(0))
        .terminateElement(elements.get(1))
        .terminateElement(elements.get(2))
        .modify();

    // then
    assertThatElementIsTerminated(processInstanceKey, "A");

    Assertions.assertThat(
            RecordingExporter.processInstanceRecords()
                .onlyEvents()
                .withProcessInstanceKey(processInstanceKey)
                .skipUntil(r -> r.getIntent() == ProcessInstanceIntent.ELEMENT_TERMINATING)
                .limitToProcessInstanceTerminated())
        .extracting(r -> r.getValue().getBpmnElementType(), Record::getIntent)
        .describedAs("Expect that all active instances of the multi-instance have been terminated")
        .containsSequence(
            tuple(BpmnElementType.MULTI_INSTANCE_BODY, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.USER_TASK, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.SUB_PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.MULTI_INSTANCE_BODY, ProcessInstanceIntent.ELEMENT_TERMINATED),
            tuple(BpmnElementType.PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATING),
            tuple(BpmnElementType.PROCESS, ProcessInstanceIntent.ELEMENT_TERMINATED));
  }

Expected behavior

No NPE should be thrown. If we reach a terminate instruction that doesn't have an existing element instance anymore we should ignore it. We already verified the this instance existed at the start of the command, so it's safe to assume another terminate instruction has already terminated the instance.

Log/Stacktrace

Full Stacktrace

15:24:38.090 [Broker-0-StreamProcessor-1] ERROR io.camunda.zeebe.processor - Expected to process record 'TypedRecordImpl{metadata=RecordMetadata{recordType=COMMAND, intentValue=255, intent=MODIFY, requestStreamId=-2147483648, requestId=-1, protocolVersion=3, valueType=PROCESS_INSTANCE_MODIFICATION, rejectionType=NULL_VAL, rejectionReason=, brokerVersion=8.1.0}, value={"processInstanceKey":2251799813685251,"terminateInstructions":[{"elementInstanceKey":2251799813685255},{"elementInstanceKey":2251799813685256},{"elementInstanceKey":2251799813685257},{"elementInstanceKey":2251799813685258}],"activateInstructions":[]}}' without errors, but exception occurred with message 'Cannot invoke "io.camunda.zeebe.engine.state.instance.ElementInstance.getValue()" because "elementInstance" is null'.
java.lang.NullPointerException: Cannot invoke "io.camunda.zeebe.engine.state.instance.ElementInstance.getValue()" because "elementInstance" is null
	at io.camunda.zeebe.engine.processing.processinstance.ProcessInstanceModificationProcessor.lambda$processRecord$3(ProcessInstanceModificationProcessor.java:201) ~[classes/:?]
	at java.lang.Iterable.forEach(Iterable.java:75) ~[?:?]
	at io.camunda.zeebe.engine.processing.processinstance.ProcessInstanceModificationProcessor.processRecord(ProcessInstanceModificationProcessor.java:197) ~[classes/:?]
	at io.camunda.zeebe.engine.Engine.process(Engine.java:127) ~[classes/:?]
	at io.camunda.zeebe.streamprocessor.ProcessingStateMachine.lambda$processCommand$3(ProcessingStateMachine.java:261) ~[classes/:?]
	at io.camunda.zeebe.db.impl.rocksdb.transaction.ZeebeTransaction.run(ZeebeTransaction.java:84) ~[classes/:?]
	at io.camunda.zeebe.streamprocessor.ProcessingStateMachine.processCommand(ProcessingStateMachine.java:257) ~[classes/:?]
	at io.camunda.zeebe.streamprocessor.ProcessingStateMachine.tryToReadNextRecord(ProcessingStateMachine.java:206) ~[classes/:?]
	at io.camunda.zeebe.streamprocessor.ProcessingStateMachine.readNextRecord(ProcessingStateMachine.java:182) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorJob.invoke(ActorJob.java:92) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorJob.execute(ActorJob.java:45) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorTask.execute(ActorTask.java:119) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorThread.executeCurrentTask(ActorThread.java:106) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorThread.doWork(ActorThread.java:87) ~[classes/:?]
	at io.camunda.zeebe.scheduler.ActorThread.run(ActorThread.java:198) ~[classes/:?]

Environment:

• OS:
• Zeebe Version:
• Configuration:

[remcowesterhoud]

@saig0 Please have a look and let me know if you agree here.