vamsizzz opened a new issue, #7125:
URL: https://github.com/apache/incubator-kie/issues/7125
---
## 1. Symptom (production)
```
java.lang.NullPointerException: Cannot invoke "java.util.Map.remove(Object)"
because "<local18>" is null
at
org.drools.core.phreak.PhreakFromNode.doLeftUpdates(PhreakFromNode.java:141)
at org.drools.core.phreak.PhreakFromNode.doNode(PhreakFromNode.java:97)
at
org.drools.core.phreak.RuleNetworkEvaluator.evalNode(RuleNetworkEvaluator.java:403)
at
org.drools.core.phreak.RuleNetworkEvaluator.innerEval(RuleNetworkEvaluator.java:349)
at
org.drools.core.phreak.RuleNetworkEvaluator.outerEval(RuleNetworkEvaluator.java:185)
at
org.drools.core.phreak.RuleNetworkEvaluator.evaluateNetwork(RuleNetworkEvaluator.java:143)
at
org.drools.core.phreak.RuleExecutor.evaluateNetwork(RuleExecutor.java:221)
at
org.drools.core.phreak.RuleExecutor.evaluateNetworkIfDirty(RuleExecutor.java:231)
at
org.drools.core.phreak.RuleExecutor.evaluateNetworkAndFire(RuleExecutor.java:76)
at
org.drools.kiesession.agenda.DefaultAgenda.fireLoop(DefaultAgenda.java:624)
...
at
org.drools.kiesession.session.StatelessKnowledgeSessionImpl.execute(StatelessKnowledgeSessionImpl.java:271)
```
It does **not** reproduce when replaying the same request directly against a
freshly-cold-loaded KJAR. It **only** reproduces when the running
`KieContainer` reaches that KJAR version via a chain of
`KieContainer.updateToVersion()` hot-reloads (e.g. v8 → v14 → v16 → v17), and
it reproduces **100% of the time** once that history exists.
---
## 2. The immediate defect: missing null-guard
`org.drools.core.phreak.PhreakFromNode.java` (stock 8.44.0.Final), method
`doLeftUpdates`, line 141:
```java
public void doLeftUpdates(FromNode fromNode,
FromMemory fm,
LeftTupleSink sink,
ReteEvaluator reteEvaluator,
TupleSets<LeftTuple> srcLeftTuples,
TupleSets<LeftTuple> trgLeftTuples,
TupleSets<LeftTuple> stagedLeftTuples) {
...
for (LeftTuple leftTuple = srcLeftTuples.getUpdateFirst(); leftTuple !=
null; ) {
LeftTuple next = leftTuple.getStagedNext();
PropagationContext propagationContext =
leftTuple.getPropagationContext();
final Map<Object, RightTuple> previousMatches = (Map<Object,
RightTuple>) leftTuple.getContextObject(); // <-- can be null
final Map<Object, RightTuple> newMatches = new HashMap<>();
leftTuple.setContextObject( newMatches );
...
for (... it.hasNext(); ) {
...
RightTuple rightTuple = previousMatches.remove(object); // <--
NPE here
...
}
for (RightTuple rightTuple : previousMatches.values()) { ... } //
<-- would also NPE
...
}
}
```
There is **zero defensive check** on `previousMatches`. The method assumes
unconditionally that whatever inserted this `LeftTuple` also called
`leftTuple.setContextObject(matches)`. That assumption is false whenever the
node was built with left-tuple memory disabled — see below.
Compare with `doLeftInserts` in the same file, which explicitly branches on
whether memory is enabled:
```java
public void doLeftInserts(FromNode fromNode, FromMemory fm, LeftTupleSink
sink,
ReteEvaluator reteEvaluator,
TupleSets<LeftTuple> srcLeftTuples,
TupleSets<LeftTuple> trgLeftTuples) {
...
for (LeftTuple leftTuple = srcLeftTuples.getInsertFirst(); leftTuple !=
null; ) {
...
Map<Object, RightTuple> matches = null;
boolean useLeftMemory = RuleNetworkEvaluator.useLeftMemory(fromNode,
leftTuple);
if (useLeftMemory) {
fm.getBetaMemory().getLeftTupleMemory().add(leftTuple);
matches = new LinkedHashMap<>();
leftTuple.setContextObject( matches ); // <-- only set when
useLeftMemory == true
}
...
}
}
```
So `doLeftInserts` correctly *skips* populating the context map when
`useLeftMemory == false`. `doLeftUpdates` never checks for that case at all.
---
## 3. Why `useLeftMemory` is false: `sequential="true"` bakes it in at
compile time
`RuleNetworkEvaluator.useLeftMemory()`:
```java
public static boolean useLeftMemory(LeftTupleSource tupleSource, Tuple
tuple) {
boolean useLeftMemory = true;
if (!tupleSource.isLeftTupleMemoryEnabled()) {
Object object = tuple.getRootTuple().getFactHandle().getObject();
if (!(object instanceof DroolsQueryImpl) || !((DroolsQueryImpl)
object).isOpen()) {
useLeftMemory = false;
}
}
return useLeftMemory;
}
```
`isLeftTupleMemoryEnabled()` on a `FromNode` traces back to a plain field
baked in once, at node-construction time, by `FromBuilder`:
```java
// org.drools.core.reteoo.builder.FromBuilder#build
FromNode fromNode = from.isReactive()
? nodeFactory.buildReactiveFromNode(context.getNextNodeId(),
from.getDataProvider(),
context.getTupleSource(), alphaNodeFieldConstraints, betaConstraints,
context.isTupleMemoryEnabled(), // <-- baked in permanently here
context, from)
: nodeFactory.buildFromNode(context.getNextNodeId(),
from.getDataProvider(),
context.getTupleSource(), alphaNodeFieldConstraints, betaConstraints,
context.isTupleMemoryEnabled(), // <-- and here
context, from);
```
And `context.isTupleMemoryEnabled()` is decided **once per rule compile,
purely from the KieBase's `sequential` flag** — nothing to do with whether that
specific pattern needs memory for joins:
```java
// org.drools.core.reteoo.builder.ReteooRuleBuilder#addRule, line ~130
if (kBase.getRuleBaseConfiguration().isSequential()) {
context.setTupleMemoryEnabled( false );
} else {
context.setTupleMemoryEnabled( true );
}
```
Confirmed against the actual production KJAR (`uswm-v17.kjar`, extracted
`kmodule.xml`):
```xml
<kbase name="Substantiation.Direct Spend or Exclusion list only"
packages="rules.substantiation.direct_spend" sequential="true">
<ksession name="Substantiation.Direct Spend or Exclusion list only"
type="stateless"/>
</kbase>
<!-- every other kbase in the module is also sequential="true" -->
```
**Sequential mode's entire premise is "insert once, fire in salience order,
never re-evaluate."** Left-tuple memory (the map that tracks "what matched last
time so I can diff on the next update") is a bookkeeping cost Drools
deliberately skips in that mode, because sequential mode's contract says
nothing should ever need to be revisited. `update()`/`modify()` calls violate
that contract for a `from`-bound fact — and Drools does not validate or reject
the combination anywhere at compile time or session time. It just leaves a
landmine.
---
## 4. Instrumented, reproducible proof (not inference)
We patched `PhreakFromNode.java` with diagnostic logging (see full patch
below), rebuilt a patched `drools-core-8.44.0.Final.jar`, put it first on the
classpath ahead of the real one, and replayed the known-poisoned hot-reload
chain `v8 → v14 → v16 → v17` against the exact production request that fails.
### Patch used for instrumentation
```java
public class PhreakFromNode {
private static final boolean DIAG =
Boolean.parseBoolean(System.getProperty("phreak.fromnode.diag",
"true"));
private static void diag(String fmt, Object... args) {
if (DIAG) System.out.println("[PHREAK-FROMNODE-DIAG] " +
String.format(fmt, args));
}
// ... doLeftInserts(): log node identity + useLeftMemory decision on
every insert
diag("INSERT node=%s tuple=%s useLeftMemory=%s existingContextObject=%s",
nodeId(fromNode), tupleId(leftTuple), useLeftMemory,
leftTuple.getContextObject() == null ? "null" :
leftTuple.getContextObject().getClass().getSimpleName());
// ... doLeftUpdates(): log + dump full diagnostic BEFORE the original
NPE fires
Object rawContextObject = leftTuple.getContextObject();
if (rawContextObject == null) {
// dumps node id, tuple, rule, dataProvider, resultClass,
reteEvaluator identity
// then falls through to the *original* unmodified cast+dereference
so the
// stack trace / behavior is unchanged for apples-to-apples
comparison with prod
}
final Map<Object, RightTuple> previousMatches = (Map<Object,
RightTuple>) rawContextObject;
```
### Result
| KJAR version | Executions on shared `FromNode` (JVM identity
`FromNode@1234d9f6`, node id `17236`) | Inserts | Updates | Result |
|---|---|---|---|---|
| v8 (cold load) | 200 | 200 | **0** | OK — never touches update path |
| v14 (`updateToVersion`) | 200 | 200 | **0** | OK |
| v16 (`updateToVersion`) | 200 | 200 | **0** | OK |
| v17 (`updateToVersion`) | 100 | 100 | **100** | **100/100 NPE** |
Every single insert across all four versions logs:
```
[PHREAK-FROMNODE-DIAG] INSERT node=FromNode@1234d9f6 id=17236
tuple=JoinNodeLeftTuple@... useLeftMemory=false existingContextObject=null
[PHREAK-FROMNODE-DIAG] INSERT node=FromNode@1234d9f6 id=17236
tuple=JoinNodeLeftTuple@... -> contextObject NOT SET (useLeftMemory=false);
leftTuple.contextObject stays null
```
The very first `UPDATE` this node has ever seen across the whole run happens
immediately after the `updateToVersion()` jump to v17, and crashes on the first
try:
```
UPDATE -> com.walmart.tender:tender-rules-kjar:1.0.17-USWM
results=[]
[PHREAK-FROMNODE-DIAG] UPDATE node=FromNode@1234d9f6 id=17236
tuple=JoinNodeLeftTuple@1d0d4478 rawContextObject=NULL (class=n/a) ...
[PHREAK-FROMNODE-DIAG] *** previousMatches IS NULL - about to NPE ***
node=FromNode@1234d9f6 id=17236
tuple=JoinNodeLeftTuple@1d0d4478
leftTuple.toString=[fact
0:1:774893764:774893764:4:DEFAULT:NON_TRAIT:...RuleContext{programApplied=true,
decisionGroup='', hasRequest=true, hasResponse=true}]
fromNode.getDataProvider=org.drools.mvel.dataproviders.MVELDataProvider@3b453e37
fromNode.getResultClass=class ...ApplyOffersRequest
reteEvaluator=KieSession[403]
FAILED com.walmart.tender:tender-rules-kjar:1.0.17-USWM#1:
java.lang.NullPointerException: Cannot invoke "java.util.Map.remove(Object)"
because "<local18>" is null
```
**Key finding:** `useLeftMemory=false` is present on *every single insert in
every version*, including v8/v14/v16 which never crash. The crash isn't caused
by the flag suddenly flipping — it's caused by v17's newly patched-in rule
content being the first version whose consequence chain actually calls
`update()` in a way that routes back through this exact,
already-memory-disabled, **shared** `FromNode` object. A from-scratch cold
compile of v17 alone does not reproduce it for the same request — only the
incremental `updateToVersion()` history does. That strongly suggests Drools'
incremental node-sharing/reuse logic during hot-reload wires new rule
consequences to pre-existing shared nodes differently (and more dangerously)
than a batch/from-scratch compile does, on top of the pre-existing
sequential-mode landmine.
### Node-sharing path checked (for completeness)
`org.drools.core.reteoo.builder.BuildUtils#attachNode` is where Drools
decides whether to reuse an existing compatible node or build a new one:
```java
} else if ( isSharingEnabledForNode(context, candidate) ) {
if ( (context.getTupleSource() != null) &&
NodeTypeEnums.isLeftTupleSink(candidate) ) {
node =
context.getTupleSource().getSinkPropagator().getMatchingNode(candidate);
}
...
}
if ( node != null && !areNodesCompatibleForSharing(context, node) ) {
node = null;
}
```
Because `tupleMemoryEnabled` is a **KieBase-wide** setting (driven purely by
`sequential`), every `FromNode` built anywhere in this kbase — whether during a
fresh compile or an incremental patch — computes the same `false` value. So
`areNodesCompatibleForSharing` never sees a mismatch to reject; sharing a
memory-disabled node across rule versions is "correct" by Drools' own
compatibility check, it's just that the underlying premise (no rule will ever
call `update()` through it) turns out to be false once v17 adds a consequence
that does.
---
## 5. Suggested fix (for the upstream ticket)
Two independent, complementary fixes:
1. **Defensive null-check in `PhreakFromNode.doLeftUpdates()`** — if
`previousMatches` is null (i.e., this node was built without left-tuple
memory), either:
- treat it as an empty map (degrade gracefully — matches
sequential-mode's "don't bother" intent), or
- throw a clear, actionable exception (`IllegalStateException:
update()/modify() is not supported for 'from' bindings in a sequential-mode
KieBase — node <id>, rule <rule>`) instead of a bare NPE.
2. **Compile-time or build-time validation** that rejects (or at least warns
on) `update()`/`modify()` calls whose propagation reaches a `from` node in a
`sequential="true"` kbase, so this is caught during `kieBuilder.buildAll()`
instead of at runtime under specific hot-reload histories.
---
## 6. Public-safe minimal repro (use this when filing the GitHub issue — do
not paste our internal package names)
```java
// kmodule.xml
// <kbase name="repro" sequential="true"><ksession name="repro"
type="stateless"/></kbase>
// Repro.drl
rule "insert-holder"
when
$ctx : Holder()
$x : String() from $ctx.items // from-binding on a mutable field
then
// no-op
end
rule "mutate-and-update"
salience 100
when
$ctx : Holder(applied == false)
then
$ctx.setApplied(true);
update($ctx); // <-- triggers doLeftUpdates() on the shared
FromNode above
end
```
```java
StatelessKieSession session = container.newStatelessKieSession("repro");
session.execute(new Holder());
// NPE: java.lang.NullPointerException at
org.drools.core.phreak.PhreakFromNode.doLeftUpdates
```
Note in the ticket that in our real-world case the crash was only observed
after several `KieContainer.updateToVersion()` hot-reloads of a growing ruleset
— a from-scratch compile of the final ruleset alone did not reproduce it, which
may point to a secondary defect in incremental node-sharing during
`updateToVersion()`.
---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
