Bug 809302

Created attachment 574727 [details]
Test case

Created attachment 574728 [details]
patch

A simple patch checking the null value

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
PRODUCT NAME:
        JBoss Enterprise BRMS
VERSION:
        5.2.0
SHORT DESCRIPTION:
       NPE while creating activations if a fact is modified by prior consequences
LONG DESCRIPTION:
       A NPE in terminal nodes can be raised if a same fact is updated in different agenda/rulefow groups
MANUAL INSTALL INSTRUCTIONS:
       To install this patch replace the following with the jars included in this patch:
       For BRMS Manager :
         $JBOSS_HOME/jboss-as/server/$PROFILE/jboss-brms.war/WEB-INF/lib/drools-core-5.2.0.BRMS.jar
       For BRMS Engine :
         The drools-core-5.2.0.BRMS.jar included with your application.
COMPATIBILITY:
 
DEPENDENCIES:
       N/A
SUPERSEDES:
       N/A
SUPERSEDED BY:
       N/A
CREATOR:
        Alessandro Lazarotti
DATE:
        04/03/2012

In the last activation, the tuple.getObject() is null at RuleTerminalNode.createActivations, so we have the NPE. It happens for Drools 5.3+ and BRMS 5.2+ (before these versions the algorithm had used the method "modifyLeftTuple" with a different implementation instead of createActivations).

Using a null checker in createActivation (look the my proposed patch attached) fixes this issue and the test attached in JBRULES-3234 works as expected. I've sent to customer a hotfix (temporary) using this fix and his code is working as well. However a better solution is check with Edson if tuple.getObject() should really be null in that point or not, maybe something wrong is happen in the new nodes that implements LeftTuple.

Customer is waiting an official patch for this issue ASAP.

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -8,10 +8,8 @@
        A NPE in terminal nodes can be raised if a same fact is updated in different agenda/rulefow groups
 MANUAL INSTALL INSTRUCTIONS:
        To install this patch replace the following with the jars included in this patch:
-       For BRMS Manager :
-         $JBOSS_HOME/jboss-as/server/$PROFILE/jboss-brms.war/WEB-INF/lib/drools-core-5.2.0.BRMS.jar
-       For BRMS Engine :
-         The drools-core-5.2.0.BRMS.jar included with your application.
+       For BRMS Manager: $JBOSS_HOME/jboss-as/server/$PROFILE/jboss-brms.war/WEB-INF/lib/drools-core-5.2.0.BRMS.jar
+       For BRMS Engine: The drools-core-5.2.0.BRMS.jar included with your application.
 COMPATIBILITY:
  
 DEPENDENCIES:

Mark Proctor <mark.proctor> made a comment on jira JBRULES-3234

The assert would get blocked, due to lock on active. So we'd have a fully matched rule with no AgendaItem. The later modify would expect there to be an AgendaItem as it's fully matched.

While i could have just done a null check, I changed this to just add a Boolean object if blocked and check for that. A null check is broader and might mask other issues.

Mark Proctor <mark.proctor> updated the status of jira JBRULES-3234 to Closed

cherry-picked from 5.4 to 5.3.x:
commit 316806a57f426dfd8d86bb764921d6d0745bc26f
Author: Mark Proctor <mproctor>
Date:   Wed Apr 4 12:12:42 2012 +0100

    JBRULES-3234 NPE on modify with lock-on-active
    (cherry picked from commit c6a3beea974234df599c4b3b80749f8ff15d3149)

    Conflicts:

        drools-compiler/src/test/java/org/drools/integrationtests/ExecutionFlowControlTest.java

commit f10be82a3e8ab62893afc7d5e6be7ffcd3262541

Fix backported into the 5.2.x branch as well.

https://github.com/droolsjbpm/drools/commit/7b86663a9d280060a8e1e778099e5e6c355240f8

Productization, please build the '.jar' patch as described in email chain 'Re: Current patches need categorization, need inputs from QE and GSS (Alessandro)' from 04/05.

We know resources are stretched for Productization at this time, please keep us advised.  Thanks much for this effort, we know things are very busy!

    Technical note updated. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    Diffed Contents:
@@ -1,3 +1,6 @@
+PATCH NAME:
+        Bug 809302 - One off patch
+        Built upon previous roll up patch BZ-787847
 PRODUCT NAME:
         JBoss Enterprise BRMS
 VERSION:
@@ -6,8 +9,9 @@
        NPE while creating activations if a fact is modified by prior consequences
 LONG DESCRIPTION:
        A NPE in terminal nodes can be raised if a same fact is updated in different agenda/rulefow groups
+
 MANUAL INSTALL INSTRUCTIONS:
-       To install this patch replace the following with the jars included in this patch:
+       To install this patch replace the following with the jar from BZ809302.zip
        For BRMS Manager: $JBOSS_HOME/jboss-as/server/$PROFILE/jboss-brms.war/WEB-INF/lib/drools-core-5.2.0.BRMS.jar
        For BRMS Engine: The drools-core-5.2.0.BRMS.jar included with your application.
 COMPATIBILITY:
@@ -19,6 +23,6 @@
 SUPERSEDED BY:
        N/A
 CREATOR:
-        Alessandro Lazarotti
+        Neil Wallace
 DATE:
-        04/03/2012+        April 10th 2012

The patch (BZ809302.zip) contains drools-core-5.2.1.BRMS.jar, but it should be drools-core-5.2.0.BRMS.jar

There is even more serious problem - this is supposed to be one-off patch, but it seems like cumulative patch (more than 50 changed class files in drools-core since 5.2.0.GA)

The patch was generated against the BRMS-P 5.2.0 roll up #1 (BZ-787847), which was
a cumulative patch.

Please, it is necessary to check if the "MANUAL INSTALL INSTRUCTIONS" will work to customer just replacing his jars or can occur some java.lang.SecurityException like "signer information does not match signer information of other classes in the same package" since the jars are not in the same compiled version.

Tks

(In reply to comment #14)
> The patch was generated against the BRMS-P 5.2.0 roll up #1 (BZ-787847), which
> was
> a cumulative patch.

There is still 7 changed class files compared to patch BZ-787847. That is more than I would expect from such a small fix.

(In reply to comment #15)
> Please, it is necessary to check if the "MANUAL INSTALL INSTRUCTIONS" will work
> to customer just replacing his jars or can occur some
> java.lang.SecurityException like "signer information does not match signer
> information of other classes in the same package" since the jars are not in the
> same compiled version.
> 
> Tks

1) BRMS Manager
changing drools-core.jar seems to be without problems

2) BRMS Engine

I tried with JBDS and it depends where Drools Library came from - if it was created from JBDS, there is SecurityException because jars created by JBDS are unsigned. On the other hand, if Drools Library was created from brms-p-5.2.0.GA-deployable/jboss-brms-engine.zip, it works.

(In reply to comment #16)
> (In reply to comment #14)
> > The patch was generated against the BRMS-P 5.2.0 roll up #1 (BZ-787847), which
> > was
> > a cumulative patch.
> 
> There is still 7 changed class files compared to patch BZ-787847. That is more
> than I would expect from such a small fix.

The real concept of "one-fix" or "single-fix" does not exist, all fixes are committed on a version of the same branch (in this case, 5.2.x
branch in github), so all fixes are now cumulative. This is different from what had occurred in the model JIRA/SVN which were separated branches to release one-off patches. Therefore you will see more changes than really used by this fix.

(In reply to comment #18)
> (In reply to comment #16)
> > (In reply to comment #14)
> > > The patch was generated against the BRMS-P 5.2.0 roll up #1 (BZ-787847), which
> > > was
> > > a cumulative patch.
> > 
> > There is still 7 changed class files compared to patch BZ-787847. That is more
> > than I would expect from such a small fix.
> 
> The real concept of "one-fix" or "single-fix" does not exist, all fixes are
> committed on a version of the same branch (in this case, 5.2.x
> branch in github), so all fixes are now cumulative. This is different from what
> had occurred in the model JIRA/SVN which were separated branches to release
> one-off patches. Therefore you will see more changes than really used by this
> fix.

Ok, thank you for the explanation, so be it. But still, file name of jar with the fix should be drools-core-5.2.0.BRMS.jar instead of drools-core-5.2.1.BRMS.jar

Hi team, customer is waiting this fix to migrate to production his application. When is the estimated date to release it?

Jars have been renamed.  QE, could we please try again?  

Thanks,

Rick

Patch is ready to be uploaded to customer portal.

Thanks

Created attachment 579910 [details]
Patch jar

MD5Sum for the attached fix .jar is 4a486cc98759a1456870e3c46fc7f678

This patch is applicable to JBoss BRMS 5.2.0. It is available for download
from the following location:
https://access.redhat.com/jbossnetwork/restricted/softwareDetail.html?softwareId=11933