tkobayas opened a new issue, #7201: URL: https://github.com/apache/incubator-kie/issues/7201
## Sequential mode semantics Sequential mode evaluates each rule exactly once, in salience order: - **Without update/modify in RHS**: all rules see the facts as they were at insert time. - **With update/modify in RHS**: a rule that has already fired does not re-evaluate. Rules not yet evaluated see the updated values when their turn comes. In other words, sequential mode guarantees that **each rule sees the current state of facts at the moment it is selected from the agenda**. Tuple memory is disabled because re-evaluation never happens. This behavior was established when PHREAK replaced RETE as the default algorithm in Drools 6.0. From the [Drools 6.0 release notes](https://docs.drools.org/6.5.0.CR1/drools-docs/html/ch02.html) (§2.13.1): > Sequential mode continues to be supported for PHREAK but now 'modify' is allowed. While there is no 'inference' with sequential configuration, as rules are lazily evaluated, any rule not yet evaluated will see the more recent data as a result of 'modify'. This is more inline with how people intuitively think sequential works. However, the current user-facing documentation (rule engine section) still carries the pre-6.0 description that says sequential mode "ignores any insert, modify, or update statements in rules." This is inaccurate for the PHREAK implementation and should be corrected as part of this issue (see #7133). ## Problem Beta node sharing violates this guarantee. When two rules share a beta node segment, evaluating the first rule pushes inserts through the **shared** segment. If that rule then calls `modify()`, the update is staged in the shared segment — meaning the second rule's beta nodes have **already been partially evaluated** (via the shared insert) before the rule is selected from the agenda. When the second rule is selected, its evaluation processes the staged update through a beta node that has no tuple memory, causing `NullPointerException` or silently producing incorrect results. This is the root cause of #7125 (NPE in `PhreakFromNode.doLeftUpdates`), and also affects `JoinNode` and potentially `AccumulateNode`, `NotNode`, and `ExistsNode`. ## Affected node types | Node type | Symptom | Location | |---|---|---| | **FromNode** (left update) | `NullPointerException`: `previousMatches` is null | `PhreakFromNode.doLeftUpdates` | | **JoinNode** (left update) | `NullPointerException`: `ltm` is null | `PhreakNodeOperations.doUpdatesReorderLeftMemory` via `PhreakJoinNode` | | **JoinNode** (right update) | Silent incorrect result: skipped by existing `ltm != null` guard (BZ-1228313) but fires with stale match | `PhreakJoinNode.doRightUpdates` | | **AccumulateNode** (left update) | Likely NPE (untested) — calls `doUpdatesReorderLeftMemory` | `PhreakAccumulateNode` | | **NotNode / ExistsNode** (left update) | Likely NPE (untested) — `doUpdatesExistentialReorderLeftMemory` calls `ltm.add` when no blocker | `PhreakNotNode` / `PhreakExistsNode` | ## Reproduction Two rules R1 (salience 100) and R2 share the same LHS pattern and beta node. R1 fires first and calls `modify()`. The update propagates through the shared segment and is processed when R2 evaluates. ### FromNode — NPE on main ```java // R1 and R2 share: Person(age >= 0) and Address() from $p.addresses // R1 (salience 100) fires first and calls modify($p) // R2's evaluation hits the staged update → NPE in doLeftUpdates String str = "package org.drools.mvel.compiler.test\n" + "import " + Person.class.getCanonicalName() + "\n" + "import " + Address.class.getCanonicalName() + "\n" + "global " + List.class.getCanonicalName() + " list\n" + "rule R1 salience 100 no-loop when\n" + " $p : Person( age >= 0 )\n" + " $a : Address() from $p.addresses\n" + "then\n" + " modify( $p ) { setAge( $p.getAge() + 1 ) };\n" + " list.add( \"R1\" );\n" + "end\n" + "rule R2 when\n" + " $p : Person( age >= 0 )\n" + " $a : Address() from $p.addresses\n" + "then\n" + " list.add( $a.getStreet() );\n" + "end\n"; ``` ### JoinNode — NPE on main ```java // R1 and R2 share: Person(age >= 0) and Cheese(type == $p.likes) // R1 (salience 100) fires first and calls modify($p) // R2's evaluation hits the staged update → NPE in doUpdatesReorderLeftMemory String str = "package org.drools.mvel.compiler.test\n" + "import " + Person.class.getCanonicalName() + "\n" + "import " + Cheese.class.getCanonicalName() + "\n" + "global " + List.class.getCanonicalName() + " list\n" + "rule R1 salience 100 no-loop when\n" + " $p : Person( age >= 0 )\n" + " $c : Cheese( type == $p.likes )\n" + "then\n" + " modify( $p ) { setAge( $p.getAge() + 1 ) };\n" + " list.add( \"R1\" );\n" + "end\n" + "rule R2 when\n" + " $p : Person( age >= 0 )\n" + " $c : Cheese( type == $p.likes )\n" + "then\n" + " list.add( \"R2:\" + $c.getType() );\n" + "end\n"; ``` ## Comparison: shared vs non-shared beta nodes With `ShareBetaNodesOption.NO`, all cases produce correct results matching non-sequential behavior: | Case | Non-sequential | Sequential + sharing ON | Sequential + sharing OFF | |---|---|---|---| | FROM: modify field not in from binding | `[R1, R2:London]` | `[R1, R2:London]` | `[R1, R2:London]` | | FROM: modify from source (London→Paris) | `[R1, R2:Paris]` | `[R1, R2:London]` (**wrong**) | `[R1, R2:Paris]` | | JOIN: modify left (Person.age) | `[R1, R2:stilton]` | **NPE** | `[R1, R2:stilton]` | | JOIN: modify right (Cheese.type) | `[R1]` | `[R1, R2:cheddar]` (**wrong**) | `[R1]` | ## Root cause The issue is not with update/modify in sequential mode per se — that is intentionally supported (DROOLS-650, BZ-1228098, BZ-1228313). The issue is that beta node sharing causes updates to reach a shared node segment before the owning rule is selected from the agenda, bypassing sequential mode's "evaluate once when selected" guarantee. Without sharing, each rule has its own beta node instances. Updates are staged against the rule's own insert and merged when the rule is selected, so the rule sees the current state at evaluation time. ## Why only beta node sharing, not alpha node sharing Alpha node sharing is not affected. Alpha nodes are stateless filters — they evaluate a constraint against the fact object itself (e.g., `Person(age >= 0)`). When a fact is modified, the update propagates through all alpha nodes, which evaluate the **current** state of the object. There is no cached or memorized result to go stale. Beta nodes are different: they join two inputs (left tuples + right input) and store the join state in tuple memory. In sequential mode, this memory is disabled (`null`). When beta node sharing causes a shared node to process an insert during one rule's evaluation, the join state for the other rule's segment is partially established — but without tuple memory, the subsequent update cannot reference or clean up that state, leading to NPE or incorrect results. In short: alpha nodes always see the new value because they evaluate the live object. Beta nodes fail because they depend on tuple memory that sequential mode disables. ## Proposed fix **Disable beta node sharing when sequential mode is enabled**, following the same pattern as eager evaluation (disabled for sequential in BZ-1228098). This should be done in `ReteooRuleBuilder` or `KnowledgeBaseImpl` — the same layer that already disables tuple memory and eager evaluation for sequential mode. ### Backward compatibility - Default behavior changes from sharing-ON to sharing-OFF only in sequential mode. - Users who rely on sharing for performance in sequential mode can opt back in via `ShareBetaNodesOption.YES` (or system property `drools.shareBetaNodes=true`). - Non-sequential mode is unaffected. - This matches the existing opt-in pattern: sequential mode already changes several defaults, and configuration options exist to override them. ### Workaround Set `drools.shareBetaNodes=false` (or `ShareBetaNodesOption.NO` in `KieBaseConfiguration`). ## Related - #7125 — original NPE report (v8.44.0.Final, `PhreakFromNode.doLeftUpdates`) - #7192 — partial fix (FromNode guard only, to be closed in favor of this issue) - #7133 — documentation update for sequential mode (user-facing docs still carry pre-6.0 description) - BZ-1228098 — precedent: eager evaluation disabled for sequential mode - BZ-1228313 — `JoinNode.doRightUpdates` guard for null `ltm` (right side only) - DROOLS-650 — established that modify works in sequential mode -- 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]
