[
https://jira.jboss.org/browse/JBRULES-2756?page=com.atlassian.jira.plugin...
]
Edson Tirelli resolved JBRULES-2756.
------------------------------------
Resolution: Done
Hi Greg,
Thanks for the excellent analysis and the help. I agree with your findings.
For a solution though, I was thinking about just completely undoing
30163 (i.e., removing that missedNotifyAll flag as I think it just
complicates things) and simply adding a halt check inside the
synchronized block that calls the wait:
synchronized ( this.halt ) {
if( !this.halt.get() ) this.halt.wait();
}
Since the notify only happens on a synchronized block, I think that
this fixes the problem as it creates a happens-before relationship
between setting the halt flag and ( notifying all threads on the halt
monitor || checking the flag and executing the wait ).
Let me know what you think.
JIRA:
https://jira.jboss.org/browse/JBRULES-2756
Changeset:
http://fisheye.jboss.org/changelog/JBossRules/?cs=35744
Thanks,
Edson
fireUntilHalt is doing busy wait and consuming 100% of one CPU core
-------------------------------------------------------------------
Key: JBRULES-2756
URL:
https://jira.jboss.org/browse/JBRULES-2756
Project: Drools
Issue Type: Bug
Security Level: Public(Everyone can see)
Components: drools-core
Affects Versions: 5.1.0.FINAL, 5.1.1.FINAL
Reporter: Edson Tirelli
Assignee: Edson Tirelli
Fix For: 5.2.0.M1
Analysis by Greg Barton:
=========================
The problem is the race condition fix introduced in rev 30163. The missedNotifyAll flag
is set to true immediately which causes the busy loop.
missedNotifyAll is initialized to false, but is immediately set to true because
notifyHalt() is called in AbstractWorkingMemory.queueWorkingMemoryAction() via
ReteooRuleBase.newStatefulSession(). Anyway, I'll move discussions of possible
solutions over to the dev list, (crossposting now) or a JIRA if you'd prefer that.
Here's a diff for the minimal change I think fixes the problem:
Index: src/main/java/org/drools/common/DefaultAgenda.java
===================================================================
--- src/main/java/org/drools/common/DefaultAgenda.java (revision 35727)
+++ src/main/java/org/drools/common/DefaultAgenda.java (working copy)
@@ -1048,10 +1048,10 @@
boolean fired = fireNextItem( agendaFilter );
fired = fired || !((AbstractWorkingMemory)
this.workingMemory).getActionQueue().isEmpty();
this.workingMemory.executeQueuedActions();
- if ( !fired && !missedNotifyAll.get()) {
+ if ( !fired ) {
try {
synchronized ( this.halt ) {
- this.halt.wait();
+ if(!missedNotifyAll.get()) this.halt.wait();
}
} catch ( InterruptedException e ) {
this.halt.set( true );
Basically, move the missedNotifyAll check immediately before the wait. This still avoids
the race condition, and only causes one spurious trip through the fireUntilHalt loop on
startup. I'd open a JIRA with this in it but I need to get to sleep. :)
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
https://jira.jboss.org/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira