[jboss-jira] [JBoss JIRA] Commented: (JBRULES-2335) StackOverflowError on serialization of KnowledgeBase

[Justin Waugh (JIRA)]

    [ https://jira.jboss.org/jira/browse/JBRULES-2335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12494399#action_12494399 ] 

Justin Waugh commented on JBRULES-2335:
---------------------------------------

Oops.. bad paste on code for Alpha Node et al.

Should be:

    public void readExternal(ObjectInput in) throws IOException,
                                            ClassNotFoundException {
        super.readExternal( in );
        constraint = (AlphaNodeFieldConstraint) in.readObject();
    }

    public void writeExternal(ObjectOutput out) throws IOException {
        super.writeExternal( out );
        out.writeObject( constraint );
    }

> StackOverflowError on serialization of KnowledgeBase
> ----------------------------------------------------
>
>                 Key: JBRULES-2335
>                 URL: https://jira.jboss.org/jira/browse/JBRULES-2335
>             Project: Drools
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: drools-core
>    Affects Versions: 5.0.1.FINAL
>         Environment: Windows, JDK 1.6
>            Reporter: Justin Waugh
>            Assignee: Mark Proctor
>         Attachments: droolstest.zip
>
>
> This is a bit of a duplicate of another issue, which I will link, however I only tested against 5.0.1 and I have a proposed solution which I believe is only applicable to 5.0.1 +
> The problem is of course, that when serializing a KnowledgeBase with a very large amount of rules, it can potentially cause a StackOverflowError on serialization.  As far as I can tell this is due to the linked lists held by ObjectSinkNodeList and LeftTupleSinkNodeList.   The problem is that they recursively serialize the list, by serializing the first node, which serializes its next node, which serializes its next node.... and so on.  This was a bit harder to solve in the 4.0+ case I think as it relied purely on the built in java serialization.  However in 5.0+ those classes all use write/readExternal to do their own serialization.  That makes it a bit easier to solve, as we can now implement an iterative serialization for the lists.
> The problem code looks like this:
> ObjectSinkNodeList (also applies to LeftTupleSinkNodeList):
>     public void readExternal(ObjectInput in) throws IOException,
>                                             ClassNotFoundException {
>         firstNode = (ObjectSinkNode) in.readObject();
>         lastNode = (ObjectSinkNode) in.readObject();
>         size = in.readInt();
> }
>  public void writeExternal(ObjectOutput out) throws IOException {
>         out.writeObject( firstNode );
>         out.writeObject( lastNode );
>         out.writeInt( size );
> }
> AlphaNode (and all others implementing ObjectSinkNode, and similarly LeftTupleSinkNode):
> public void readExternal(ObjectInput in) throws IOException,
>                                             ClassNotFoundException {
>         super.readExternal( in );
>         constraint = (AlphaNodeFieldConstraint) in.readObject();
>         previousRightTupleSinkNode = (ObjectSinkNode) in.readObject();
>         nextRightTupleSinkNode = (ObjectSinkNode) in.readObject();
>     }
>     public void writeExternal(ObjectOutput out) throws IOException {
>         super.writeExternal( out );
>         out.writeObject( constraint );
>         out.writeObject( previousRightTupleSinkNode );
>         out.writeObject( nextRightTupleSinkNode );
>     }
> As you can see it recursively serializes the list, so the stack depth increases linearly with the list length.
> The solution is this:
> ObjectSinkNodeList (also applies to LeftTupleSinkNodeList):
>  public void readExternal(ObjectInput in) throws IOException,
>                                             ClassNotFoundException {
>         firstNode = (ObjectSinkNode) in.readObject();
>         lastNode = (ObjectSinkNode) in.readObject();
>         size = in.readInt();
>         
>         ObjectSinkNode current = firstNode;
> 		while(current != null)
> 		{
> 			ObjectSinkNode previous = (ObjectSinkNode) in.readObject();
> 			ObjectSinkNode next = (ObjectSinkNode) in.readObject();
> 			current.setPreviousObjectSinkNode(previous);
> 			current.setNextObjectSinkNode(next);
> 			current = next;
> 		}
>     }
>     public void writeExternal(ObjectOutput out) throws IOException {
>         out.writeObject( firstNode );
>         out.writeObject( lastNode );
>         out.writeInt( size );
>         
> 		for (ObjectSinkNode node = firstNode; node != null; node = node.getNextObjectSinkNode())
> 		{
> 			out.writeObject(node.getPreviousObjectSinkNode());
> 			out.writeObject(node.getNextObjectSinkNode());
> 		}
>     }
> AlphaNode (and all others implementing ObjectSinkNode, and similarly LeftTupleSinkNode):
>     public void writeExternal(ObjectOutput out) throws IOException {
>         super.writeExternal( out );
>         out.writeObject( constraint );
>     }
>     public AlphaNodeFieldConstraint getConstraint() {
>         return this.constraint;
>     }
> As you can see the responsibility for serializing the list node links is now placed into the list objects themselves, and is performed iteratively.  I suppose technically the lastNode reference of the list doesn't need to be stored separately, but it was just easier code.  
> The obvious concern here is if for some reason you had linked nodes which were not held by one of the list objects, then the links would be lost.  However I checked references to the get/set of the prev/next, and they were only referenced by the list objects themselves, so I believe there is no chance for that to happen.
> I'll attach a test case (but it's just the one from the linked ticket)

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira