This is an automated email from the ASF dual-hosted git repository.
yesamer pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/main by this push:
new b4bef4fe5f [incubator-kie-drools-6422] Fix NPE in
RuleExecutor.removeDormantTuple (#6707)
b4bef4fe5f is described below
commit b4bef4fe5f2bf44217c9577c471823fb0cc73975
Author: Toni Rikkola <[email protected]>
AuthorDate: Fri May 29 12:43:56 2026 +0300
[incubator-kie-drools-6422] Fix NPE in RuleExecutor.removeDormantTuple
(#6707)
Add a guard in removeDormantTuple to verify the tuple is actually in
the dormant list before attempting removal. The NPE likely occurs when a
tuple is removed from active matches with stagedType DELETE (skipping
dormant addition), then later processed by doLeftDelete or
modifyActiveTuple which unconditionally call removeDormantTuple.
Pin the fix with a focused unit test in drools-core that constructs
the exact list state the guard protects against: a tuple unlinked
from dormantMatches (previous == null, not the head) passed back to
removeDormantTuple. Without the guard this NPEs at
LinkedList.remove:178; with it the call is a no-op.
Coverage also includes modifyActiveTuple as a second entry point that
routes through removeDormantTuple, and the upstream precondition that
removeActiveTuple with stagedType=DELETE must skip the dormant
transition (plus its non-DELETE baseline).
Assited-By: Claude Opus 4.6 (1M context) <[email protected]>
---
.../java/org/drools/core/phreak/RuleExecutor.java | 7 +-
.../core/phreak/RuleExecutorDormantTupleTest.java | 142 +++++++++++++++++++++
2 files changed, 148 insertions(+), 1 deletion(-)
diff --git a/drools-core/src/main/java/org/drools/core/phreak/RuleExecutor.java
b/drools-core/src/main/java/org/drools/core/phreak/RuleExecutor.java
index 431e966c07..7839ea8813 100644
--- a/drools-core/src/main/java/org/drools/core/phreak/RuleExecutor.java
+++ b/drools-core/src/main/java/org/drools/core/phreak/RuleExecutor.java
@@ -311,7 +311,12 @@ public class RuleExecutor {
throw new IllegalStateException();
}
}
- dormantMatches.remove(tuple);
+ if (tuple.getPrevious() != null || dormantMatches.getFirst() == tuple)
{
+ dormantMatches.remove(tuple);
+ } else {
+ log.debug("Skipping removeDormantTuple for orphan tuple {} on rule
\"{}\" — tuple is not present in dormantMatches",
+ tuple, ruleAgendaItem.getRule().getName());
+ }
if (DEBUG_DORMANT_TUPLE) {
tuple.setDormant(false);
}
diff --git
a/drools-core/src/test/java/org/drools/core/phreak/RuleExecutorDormantTupleTest.java
b/drools-core/src/test/java/org/drools/core/phreak/RuleExecutorDormantTupleTest.java
new file mode 100644
index 0000000000..29e3a8450a
--- /dev/null
+++
b/drools-core/src/test/java/org/drools/core/phreak/RuleExecutorDormantTupleTest.java
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.core.phreak;
+
+import org.drools.base.base.SalienceInteger;
+import org.drools.base.definitions.rule.impl.RuleImpl;
+import org.drools.core.reteoo.RuleTerminalNodeLeftTuple;
+import org.drools.core.reteoo.Tuple;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Targeted regression test for the guard in {@link
RuleExecutor#removeDormantTuple} added for
+ * issue 6422. A tuple passed to {@code removeDormantTuple} may already have
been unlinked from
+ * {@code dormantMatches} by another path (e.g. {@code modifyActiveTuple}), in
which case its
+ * {@code previous} pointer is null and the unguarded {@code
LinkedList.remove()} dereferences it.
+ */
+public class RuleExecutorDormantTupleTest {
+
+ @Test
+ public void
removeDormantTuple_whenTupleAlreadyUnlinkedFromList_doesNotNPE() {
+ final RuleExecutor executor = newRuleExecutor();
+ final RuleTerminalNodeLeftTuple t1 = new RuleTerminalNodeLeftTuple();
+ final RuleTerminalNodeLeftTuple t2 = new RuleTerminalNodeLeftTuple();
+
+ executor.addDormantTuple(t1);
+ executor.addDormantTuple(t2);
+
+ // First remove takes t1 out cleanly via the firstNode path. After
this,
+ // t1.previous is null and dormantMatches.getFirst() is t2 — the exact
state
+ // the bug fix guards against.
+ executor.removeDormantTuple(t1);
+ assertThat(executor.getDormantMatches().size()).isEqualTo(1);
+ assertThat(t1.getPrevious()).isNull();
+ assertThat(executor.getDormantMatches().getFirst()).isNotSameAs(t1);
+
+ // Without the guard, LinkedList.remove() would fall into the
middle-node
+ // branch and NPE on t1.getPrevious().setNext(...).
+ assertThatNoException().isThrownBy(() ->
executor.removeDormantTuple(t1));
+
+ // The list is unchanged: t2 is still the sole dormant tuple.
+ assertThat(executor.getDormantMatches().size()).isEqualTo(1);
+ assertThat(executor.getDormantMatches().getFirst()).isSameAs(t2);
+ }
+
+ @Test
+ public void removeDormantTuple_whenTupleIsInList_unlinksIt() {
+ final RuleExecutor executor = newRuleExecutor();
+ final RuleTerminalNodeLeftTuple t1 = new RuleTerminalNodeLeftTuple();
+
+ executor.addDormantTuple(t1);
+ assertThat(executor.getDormantMatches().size()).isEqualTo(1);
+
+ executor.removeDormantTuple(t1);
+ assertThat(executor.getDormantMatches().size()).isZero();
+ }
+
+ /**
+ * {@link RuleExecutor#modifyActiveTuple} routes through {@code
removeDormantTuple}, so the
+ * same NPE could surface from this second entry point. Pins the guard
from both call sites.
+ */
+ @Test
+ public void
modifyActiveTuple_whenTupleAlreadyUnlinkedFromDormantList_doesNotNPE() {
+ final RuleExecutor executor = newRuleExecutor();
+ final RuleTerminalNodeLeftTuple t1 = new RuleTerminalNodeLeftTuple();
+ final RuleTerminalNodeLeftTuple t2 = new RuleTerminalNodeLeftTuple();
+
+ executor.addDormantTuple(t1);
+ executor.addDormantTuple(t2);
+ executor.removeDormantTuple(t1);
+
+ assertThatNoException().isThrownBy(() ->
executor.modifyActiveTuple(t1));
+
+ assertThat(executor.getActiveMatches().size()).isEqualTo(1);
+ assertThat(executor.getDormantMatches().size()).isEqualTo(1);
+ assertThat(executor.getDormantMatches().getFirst()).isSameAs(t2);
+ assertThat(t1.isQueued()).isTrue();
+ }
+
+ /**
+ * Pins the upstream half of the bug: {@link
RuleExecutor#removeActiveTuple} must skip
+ * {@code addDormantTuple} when the staged type is DELETE. This is the
precondition that
+ * leaves a tuple unlinked from {@code dormantMatches} while later paths
still try to
+ * remove it.
+ */
+ @Test
+ public void removeActiveTuple_whenStagedForDelete_skipsDormantTransition()
{
+ final RuleExecutor executor = newRuleExecutor();
+ final RuleTerminalNodeLeftTuple t1 = new RuleTerminalNodeLeftTuple();
+
+ executor.addActiveTuple(t1);
+ t1.setStagedType(Tuple.DELETE);
+
+ executor.removeActiveTuple(t1);
+
+ assertThat(executor.getActiveMatches().size()).isZero();
+ assertThat(executor.getDormantMatches().size()).isZero();
+ }
+
+ @Test
+ public void
removeActiveTuple_whenNotStagedForDelete_transitionsToDormant() {
+ final RuleExecutor executor = newRuleExecutor();
+ final RuleTerminalNodeLeftTuple t1 = new RuleTerminalNodeLeftTuple();
+
+ executor.addActiveTuple(t1);
+ // Default stagedType is NONE (0); leave it as-is.
+
+ executor.removeActiveTuple(t1);
+
+ assertThat(executor.getActiveMatches().size()).isZero();
+ assertThat(executor.getDormantMatches().size()).isEqualTo(1);
+ assertThat(executor.getDormantMatches().getFirst()).isSameAs(t1);
+ }
+
+ private static RuleExecutor newRuleExecutor() {
+ final RuleImpl rule = mock(RuleImpl.class);
+ when(rule.getSalience()).thenReturn(SalienceInteger.DEFAULT_SALIENCE);
+ final RuleAgendaItem item = mock(RuleAgendaItem.class);
+ when(item.getRule()).thenReturn(rule);
+ return new RuleExecutor(null, item, false);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
