[jboss-jira] [JBoss JIRA] (JBRULES-3100) Long.MAX_VALUE duration for "A and not(B after A)" type rules causes invalid session clock time in rule RHS when running with pseudo clock

Long.MAX_VALUE duration for "A and not(B after A)" type rules causes invalid session clock time in rule RHS when running with pseudo clock ------------------------------------------------------------------------------------------------------------------------------------------ Key: JBRULES-3100 URL: https://issues.jboss.org/browse/JBRULES-3100 Project: Drools Issue Type: Bug Security Level: Public(Everyone can see) Components: drools-core, drools-core (fusion) Affects Versions: 5.2.0.Final Reporter: Richard Calmbach Assignee: Edson Tirelli Priority: Critical Fix For: 5.4.0.Beta2 Ok, this is a subtle one. How did I run into this one? I need dynamic timers that are started in the RHS because Drools LHS syntax does not support dynamic timers. In order to be able to unit test my rules even with dynamic timers, I rely on the session clock (aka TimerService) to schedule and execute these manually defined jobs. Works beautifully with the real-time clock for all my rules, and also works with the pseudo clock for rules that don't have a negated unbounded "after" clause. However, the pseudo clock breaks the timers that are scheduled in the RHS of rules that have a LHS of type "A and not(B after A)". The reason is extremely subtle, and understanding it required an excursion into the inner workings of the rule engine, in particular its mechanism for scheduling activations of rules with non-zero duration. (Good thing that javadoc and sources are now available via Maven repositories for all Drools artifacts. Thank you, Drools team and the m2eclipse "Download Artifact Sources" checkbox!) In PseudoClockScheduler.runCallBacks(), the pseudo clock checks whether any scheduled jobs need to be executed by comparing their trigger times to the current time (this.timer). For every job that is due, e.g., a scheduled rule activation, the pseudo clock saves this.timer to a local variable (savedTimer) and sets this.timer temporarily to the trigger time of the job (keeping this.timer at that value for the duration of the job execution, e.g., the firing of the scheduled activation). Now, for rules with a bounded duration (e.g., "A and not(B after[0s, 10s] A"), this trigger time is a reasonable value (current time + duration), and everything works out. However, rules of type "A and not(B after A)" (i.e., with negated unbounded after) have a duration value of Long.MAX_VALUE. When DurationTimer.createTrigger() constructs a new PointInTimeTrigger object, it sets the trigger time to "current time + duration", and with a post-epoch current time (i.e., greater than zero) and duration == Long.MAX_VALUE, this causes wrap-around and sets the trigger time to a very large negative number. Up to this point, I think the behavior of the code is intentional (albeit hacky), to ensure that "negated unbounded after" type rules get activated right away (as they should). However, this approach has unintended, buggy consequences as we shall see. When such a scheduled rule fires, then for the duration of its RHS execution, the session clock has an invalid value (very large negative number). The only way how you would know that is if you queried the session clock, say because you're trying to schedule a dynamic timer because Drools LHS syntax only supports static timers. The upshot is that the dynamic timer job is scheduled with "very large negative number + my trigger delay" as the trigger time, which PseudoClockScheduler.runCallBacks() promptly interprets as a job that is due for execution, causing premature and therefore incorrect triggering of my dynamic timer, thereby breaking my unit tests, while the real-time clock does not exhibit this problem. And that's how I spent my Friday. I can work around this bug by rewriting the "A and not(B after A)" construct so that I'm explicitly comparing the timestamp fields instead of using "after". However, the current Drools implementation is broken in more than one way: Say, you're simulating the events at the original Woodstock (or any other events pre-dating the Unix epoch), and you're setting the pseudo clock to a negative value so that Date.toString() gives you the actual dates of the simulated events. As long as current time is a non-positive value, adding duration == Long.MAX_VALUE will yield a time much, much later than current time. This means that "A and not(B after A)" type rules will not fire when they should because their trigger time is in a future far, far away that will never be reached during the simulation. If I interpret this correctly, this also affects rule activation with the real-time clock if it is set to a negative value (there goes your time traveler market share...). I'm not sure what the best fix for these bugs is. A real fix probably has to rework how "negated unbounded after" type rule activations get scheduled. The duration == Long.MAX_VALUE hack isn't going to cut it. Maybe these rules don't need scheduling at all since their activations should be created right away? Anyway, tricky stuff.