This is an automated email from the ASF dual-hosted git repository.
tkobayas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie.git
The following commit(s) were added to refs/heads/main by this push:
new b8ca2754b70 [incubator-kie#2412] Release dynamic facts when they are
deleted or e… (#7176)
b8ca2754b70 is described below
commit b8ca2754b709b47bb7a1a39c218151d6cad90e3a
Author: Thebas <[email protected]>
AuthorDate: Tue Oct 13 09:41:04 2026 +0100
[incubator-kie#2412] Release dynamic facts when they are deleted or e…
(#7176)
* [incubator-kie#2412] Release dynamic facts when they are deleted or expire
A fact inserted with the dynamic flag, insert(obj, true), is recorded in
NamedEntryPoint.dynamicFacts and has the entry point registered as its
PropertyChangeListener. Deleting it only undid that when the fact's type was
declared dynamic, so for any other type the handle, the fact and the
listener
stayed referenced until the session was disposed. A long-running session
that
keeps inserting and deleting dynamic facts grows until OutOfMemoryError.
Release the listener whenever the fact is dynamic, either through its type
or
through the insert flag. Do the same when an event expires, which removed
the
handle from the object store without releasing the listener at all. Logical
inserts never register a listener, so the TMS paths need no change.
A class annotated with @PropertyChangeSupport now makes its type dynamic,
the
same as declaring it with @propertyChangeSupport in DRL. Before, only the
DRL
declaration had that effect and the class annotation was ignored.
* [incubator-kie#2412] Guard dynamicFacts with the entry point lock on
expiry
insert() adds to dynamicFacts while holding the entry point lock, but an
expiring event reaches removeFromObjectStore() from the agenda without it,
so releasing its listener could touch the HashSet concurrently with an
insert on another thread. Take the lock around the listener release there;
the delete path already runs under it.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---------
Co-authored-by: MARTINS FERRAZ Andre <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
.../java/org/drools/base/rule/TypeDeclaration.java | 9 ++
.../kiesession/entrypoints/NamedEntryPoint.java | 18 ++-
.../PropertyChangeSupportTest.java | 153 ++++++++++++++++++++-
3 files changed, 176 insertions(+), 4 deletions(-)
diff --git
a/drools-base/src/main/java/org/drools/base/rule/TypeDeclaration.java
b/drools-base/src/main/java/org/drools/base/rule/TypeDeclaration.java
index 195bed98f99..a71b1eb294b 100644
--- a/drools-base/src/main/java/org/drools/base/rule/TypeDeclaration.java
+++ b/drools-base/src/main/java/org/drools/base/rule/TypeDeclaration.java
@@ -37,6 +37,7 @@ import org.drools.util.ClassUtils;
import org.kie.api.definition.type.ClassReactive;
import org.kie.api.definition.type.Expires;
import org.kie.api.definition.type.Expires.Policy;
+import org.kie.api.definition.type.PropertyChangeSupport;
import org.kie.api.definition.type.PropertyReactive;
import org.kie.api.definition.type.Role;
import org.kie.api.io.Resource;
@@ -564,6 +565,7 @@ public class TypeDeclaration
public static void processTypeAnnotations( TypeDeclaration type, Annotated
annotated, PropertySpecificOption propertySpecificOption ) {
configureExpirationOffset( type, annotated );
configurePropertyReactivity( type, annotated, propertySpecificOption );
+ configurePropertyChangeSupport( type, annotated );
}
private static void configureExpirationOffset( TypeDeclaration type,
Annotated annotated ) {
@@ -582,4 +584,11 @@ public class TypeDeclaration
annotated.hasAnnotation( ClassReactive.class ) );
type.setPropertyReactive(propertyReactive);
}
+
+ private static void configurePropertyChangeSupport( TypeDeclaration type,
Annotated annotated ) {
+ // only ever switch it on: a DRL declaration may already have made the
type dynamic
+ if ( annotated.hasAnnotation( PropertyChangeSupport.class ) ) {
+ type.setDynamic( true );
+ }
+ }
}
diff --git
a/drools-kiesession/src/main/java/org/drools/kiesession/entrypoints/NamedEntryPoint.java
b/drools-kiesession/src/main/java/org/drools/kiesession/entrypoints/NamedEntryPoint.java
index d48be8afe77..874e9d7ec48 100644
---
a/drools-kiesession/src/main/java/org/drools/kiesession/entrypoints/NamedEntryPoint.java
+++
b/drools-kiesession/src/main/java/org/drools/kiesession/entrypoints/NamedEntryPoint.java
@@ -480,9 +480,7 @@ public class NamedEntryPoint implements
InternalWorkingMemoryEntryPoint, Propert
final ObjectTypeConf typeConf =
getObjectTypeConfigurationRegistry().getObjectTypeConf( object );
- if( typeConf.isDynamic() ) {
- removePropertyChangeListener( handle, true );
- }
+ removeDynamicPropertyChangeListener( handle, typeConf );
PropagationContext propagationContext = delete( handle, object,
typeConf, rule, terminalNode );
@@ -543,9 +541,23 @@ public class NamedEntryPoint implements
InternalWorkingMemoryEntryPoint, Propert
public void removeFromObjectStore(InternalFactHandle handle) {
this.objectStore.removeHandle( handle );
ObjectTypeConf typeConf =
getObjectTypeConfigurationRegistry().getObjectTypeConf( handle.getObject() );
+ // expiration reaches here without the entry point lock, which guards
dynamicFacts
+ lock();
+ try {
+ removeDynamicPropertyChangeListener( handle, typeConf );
+ } finally {
+ unlock();
+ }
deleteFromTMS( handle, handle.getEqualityKey(), typeConf, null );
}
+ private void removeDynamicPropertyChangeListener( InternalFactHandle
handle, ObjectTypeConf typeConf ) {
+ // a fact is dynamic either because its type is, or because it was
inserted with the dynamic flag
+ if ( typeConf.isDynamic() || ( dynamicFacts != null &&
dynamicFacts.contains( handle ) ) ) {
+ removePropertyChangeListener( handle, true );
+ }
+ }
+
public void addPropertyChangeListener(final InternalFactHandle handle,
final boolean dynamicFlag ) {
Object object = handle.getObject();
try {
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/PropertyChangeSupportTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/PropertyChangeSupportTest.java
index f64c2c99db4..0d6119e84f5 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/PropertyChangeSupportTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/PropertyChangeSupportTest.java
@@ -20,10 +20,22 @@ package org.drools.compiler.integrationtests;
import java.beans.PropertyChangeListener;
import java.beans.PropertyChangeSupport;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
+import org.kie.api.KieBase;
+import org.kie.api.KieServices;
+import org.kie.api.conf.EventProcessingOption;
+import org.kie.api.definition.type.Expires;
+import org.kie.api.definition.type.Role;
import org.kie.api.io.ResourceType;
import org.kie.api.runtime.KieSession;
+import org.kie.api.runtime.KieSessionConfiguration;
+import org.kie.api.runtime.conf.ClockTypeOption;
+import org.kie.api.runtime.rule.FactHandle;
+import org.kie.api.time.SessionPseudoClock;
import org.kie.internal.utils.KieHelper;
import static org.assertj.core.api.Assertions.assertThat;
@@ -62,6 +74,32 @@ public class PropertyChangeSupportTest {
public void removePropertyChangeListener(PropertyChangeListener
listener) {
support.removePropertyChangeListener(listener);
}
+
+ public int getListenerCount() {
+ return support.getPropertyChangeListeners().length;
+ }
+ }
+
+ @org.kie.api.definition.type.PropertyChangeSupport
+ public static class AnnotatedDynamicFact extends DynamicFact {
+ }
+
+ @org.kie.api.definition.type.PropertyChangeSupport
+ @Role(Role.Type.EVENT)
+ @Expires("1s")
+ public static class ExpiringDynamicFact extends DynamicFact {
+ }
+
+ public static class Trigger {
+ private final String name;
+
+ public Trigger(String name) {
+ this.name = name;
+ }
+
+ public String getName() {
+ return name;
+ }
}
@Test
@@ -99,4 +137,117 @@ public class PropertyChangeSupportTest {
assertThat(fact.getValue()).isEqualTo("VAL1");
}
-}
\ No newline at end of file
+
+ @Test
+ public void testDeleteUnregistersFactInsertedAsDynamic() {
+ // a fact inserted with the dynamic flag must be released on delete,
even if its type isn't dynamic
+ String drl =
+ "import " + DynamicFact.class.getCanonicalName() + ";\n" +
+ "import " + Trigger.class.getCanonicalName() + ";\n" +
+ "global java.util.List facts\n" +
+ "rule insertDynamic\n" +
+ " when\n" +
+ " $t: Trigger()\n" +
+ " then\n" +
+ " delete($t);\n" +
+ " DynamicFact fact = new DynamicFact();\n" +
+ " fact.setName($t.getName());\n" +
+ " facts.add(fact);\n" +
+ " insert(fact, true);\n" +
+ " end\n" +
+ " \n" +
+ " rule deleteDynamic\n" +
+ " when\n" +
+ " $fact: DynamicFact()\n" +
+ " then\n" +
+ " delete($fact);\n" +
+ " end";
+
+ KieSession ksession = new KieHelper().addContent(drl,
ResourceType.DRL).build().newKieSession();
+ try {
+ List<DynamicFact> facts = new ArrayList<>();
+ ksession.setGlobal("facts", facts);
+
+ for (int i = 0; i < 10; i++) {
+ ksession.insert(new Trigger("t" + i));
+ ksession.fireAllRules();
+ }
+
+ assertThat(ksession.getFactCount()).isZero();
+ assertThat(facts).hasSize(10).allSatisfy(fact ->
assertThat(fact.getListenerCount()).isZero());
+ } finally {
+ ksession.dispose();
+ }
+ }
+
+ @Test
+ public void testClassLevelAnnotationMakesTypeDynamic() {
+ // the class annotation must work like declaring the type with
@propertyChangeSupport in DRL
+ String drl =
+ "import " + AnnotatedDynamicFact.class.getCanonicalName() +
";\n" +
+ "rule rule1\n" +
+ " when\n" +
+ " $fact: AnnotatedDynamicFact(name == \"user1\")\n" +
+ " then\n" +
+ " $fact.setName(\"user2\");\n" +
+ " end\n" +
+ " \n" +
+ " rule rule2\n" +
+ " when\n" +
+ " $fact: AnnotatedDynamicFact(name == \"user2\")\n" +
+ " then\n" +
+ " $fact.setValue($fact.getValue() + \"VAL1\");\n" +
+ " end";
+
+ KieSession ksession = new KieHelper().addContent(drl,
ResourceType.DRL).build().newKieSession();
+ try {
+ AnnotatedDynamicFact fact = new AnnotatedDynamicFact();
+ fact.setName("user1");
+ fact.setValue("");
+
+ FactHandle handle = ksession.insert(fact);
+ assertThat(fact.getListenerCount()).isEqualTo(1);
+
+ ksession.fireAllRules(10);
+
+ assertThat(fact.getName()).isEqualTo("user2");
+ assertThat(fact.getValue()).isEqualTo("VAL1");
+
+ ksession.delete(handle);
+ assertThat(fact.getListenerCount()).isZero();
+ } finally {
+ ksession.dispose();
+ }
+ }
+
+ @Test
+ public void testExpiryUnregistersDynamicFact() {
+ String drl =
+ "import " + ExpiringDynamicFact.class.getCanonicalName() +
";\n" +
+ "rule rule1\n" +
+ " when\n" +
+ " ExpiringDynamicFact()\n" +
+ " then\n" +
+ " end";
+
+ KieBase kbase = new KieHelper().addContent(drl,
ResourceType.DRL).build(EventProcessingOption.STREAM);
+ KieSessionConfiguration ksconf =
KieServices.get().newKieSessionConfiguration();
+ ksconf.setOption(ClockTypeOption.PSEUDO);
+ KieSession ksession = kbase.newKieSession(ksconf, null);
+ try {
+ ExpiringDynamicFact fact = new ExpiringDynamicFact();
+ ksession.insert(fact);
+ ksession.fireAllRules();
+ assertThat(fact.getListenerCount()).isEqualTo(1);
+
+ SessionPseudoClock clock = (SessionPseudoClock)
ksession.getSessionClock();
+ clock.advanceTime(2, TimeUnit.SECONDS);
+ ksession.fireAllRules();
+
+ assertThat(ksession.getFactCount()).isZero();
+ assertThat(fact.getListenerCount()).isZero();
+ } finally {
+ ksession.dispose();
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
