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

StackOverflowError on serialization of KnowledgeBase ---------------------------------------------------- Key: JBRULES-2335 URL: https://issues.jboss.org/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 Fix For: 5.2.0.CR1 Attachments: droolstest.zip, stackoverflowfix-r30647.patch 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)