JENA-904: Fix for LPBRuleEngine leaking activeInterpreters
Project: http://git-wip-us.apache.org/repos/asf/jena/repo Commit: http://git-wip-us.apache.org/repos/asf/jena/commit/4edc6d38 Tree: http://git-wip-us.apache.org/repos/asf/jena/tree/4edc6d38 Diff: http://git-wip-us.apache.org/repos/asf/jena/diff/4edc6d38 Branch: refs/heads/add-contract-tests Commit: 4edc6d38613970894165565000f9d1f1a50a6e4c Parents: 84d0615 Author: Andy Seaborne <[email protected]> Authored: Fri May 1 12:56:24 2015 +0100 Committer: Andy Seaborne <[email protected]> Committed: Fri May 1 12:56:24 2015 +0100 ---------------------------------------------------------------------- .../rulesys/impl/ConsumerChoicePointFrame.java | 11 ++ .../jena/reasoner/rulesys/impl/FrameObject.java | 5 +- .../reasoner/rulesys/impl/LPInterpreter.java | 3 +- .../rulesys/impl/LPTopGoalIterator.java | 1 + .../rulesys/impl/TopLevelTripleMatchFrame.java | 2 + .../reasoner/rulesys/impl/TripleMatchFrame.java | 1 + .../rulesys/impl/TestLPBRuleEngineLeak.java | 171 +++++++++++++++++++ .../jena/reasoner/rulesys/test/TestPackage.java | 4 + 8 files changed, 196 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/ConsumerChoicePointFrame.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/ConsumerChoicePointFrame.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/ConsumerChoicePointFrame.java index e0d0b85..a1c2c26 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/ConsumerChoicePointFrame.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/ConsumerChoicePointFrame.java @@ -161,6 +161,17 @@ public class ConsumerChoicePointFrame extends GenericTripleMatchFrame context.setReady(this); } + @Override + public void close() { + super.close(); + if (generator != null && generator.interpreter != null) { + generator.interpreter.close(); + // .. but NOT + //generator.setComplete(); + // as it seems to cause other tests to fail + } + } + /** * Notify that this consumer choice point has finished consuming all * the results of a closed generator. http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/FrameObject.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/FrameObject.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/FrameObject.java index 39ccc52..4128d36 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/FrameObject.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/FrameObject.java @@ -54,7 +54,10 @@ public class FrameObject { * Close the frame actively. This frees any internal resources, frees this frame and * frees the frame to which this is linked. */ - public void close() { + public void close() { + if (link != null) { + link.close(); + } } } http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPInterpreter.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPInterpreter.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPInterpreter.java index 3edbb6d..fee6c97 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPInterpreter.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPInterpreter.java @@ -164,8 +164,9 @@ public class LPInterpreter { */ public void close() { isComplete = true; - engine.detach(this); if (cpFrame != null) cpFrame.close(); + engine.detach(this); + } /** http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPTopGoalIterator.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPTopGoalIterator.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPTopGoalIterator.java index cb14070..d4d471b 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPTopGoalIterator.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/LPTopGoalIterator.java @@ -197,6 +197,7 @@ public class LPTopGoalIterator implements ClosableIterator<Triple>, LPInterprete // Check for any dangling generators which are complete interpreter.getEngine().checkForCompletions(); + interpreter.close(); // Close this top goal lookAhead = null; //LogFactory.getLog( getClass() ).debug( "Nulling and closing LPTopGoalIterator " + interpreter ); http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TopLevelTripleMatchFrame.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TopLevelTripleMatchFrame.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TopLevelTripleMatchFrame.java index 80a718d..0caf8b2 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TopLevelTripleMatchFrame.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TopLevelTripleMatchFrame.java @@ -66,6 +66,8 @@ public class TopLevelTripleMatchFrame extends GenericChoiceFrame { @Override public void close() { if (matchIterator != null) matchIterator.close(); + super.close(); + } } http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TripleMatchFrame.java ---------------------------------------------------------------------- diff --git a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TripleMatchFrame.java b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TripleMatchFrame.java index 8d4891d..74b1a1b 100644 --- a/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TripleMatchFrame.java +++ b/jena-core/src/main/java/org/apache/jena/reasoner/rulesys/impl/TripleMatchFrame.java @@ -73,6 +73,7 @@ public class TripleMatchFrame extends GenericTripleMatchFrame { */ @Override public void close() { if (matchIterator != null) matchIterator.close(); + super.close(); } } http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/impl/TestLPBRuleEngineLeak.java ---------------------------------------------------------------------- diff --git a/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/impl/TestLPBRuleEngineLeak.java b/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/impl/TestLPBRuleEngineLeak.java new file mode 100644 index 0000000..58e0afb --- /dev/null +++ b/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/impl/TestLPBRuleEngineLeak.java @@ -0,0 +1,171 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.jena.reasoner.rulesys.impl; + +import java.lang.reflect.Field; +import java.util.List; + +import junit.framework.TestCase; +import junit.framework.TestSuite; + +import org.junit.Test; + +import org.apache.jena.graph.Factory; +import org.apache.jena.graph.Graph; +import org.apache.jena.graph.Node; +import org.apache.jena.graph.NodeFactory; +import org.apache.jena.graph.Triple; +import org.apache.jena.reasoner.rulesys.FBRuleInfGraph; +import org.apache.jena.reasoner.rulesys.FBRuleReasoner; +import org.apache.jena.reasoner.rulesys.Rule; +import org.apache.jena.util.iterator.ExtendedIterator; +import org.apache.jena.vocabulary.RDF; +import org.apache.jena.vocabulary.RDFS; + +public class TestLPBRuleEngineLeak extends TestCase { + public static TestSuite suite() { + return new TestSuite(TestLPBRuleEngineLeak.class, "TestLPBRuleEngineLeak"); + } + + protected Node a = NodeFactory.createURI("a"); + protected Node b = NodeFactory.createURI("b"); + protected Node nohit = NodeFactory.createURI("nohit"); + protected Node p = NodeFactory.createURI("p"); + protected Node C1 = NodeFactory.createURI("C1"); + protected Node C2 = NodeFactory.createURI("C2"); + protected Node ty = RDF.Nodes.type; + + public FBRuleReasoner createReasoner(List<Rule> rules) { + FBRuleReasoner reasoner = new FBRuleReasoner(rules); + reasoner.tablePredicate(RDFS.Nodes.subClassOf); + reasoner.tablePredicate(RDF.Nodes.type); + reasoner.tablePredicate(p); + return reasoner; + } + + @Test + public void testNotLeakingActiveInterpreters() throws Exception { + Graph data = Factory.createGraphMem(); + data.add(new Triple(a, ty, C1)); + data.add(new Triple(b, ty, C1)); + List<Rule> rules = Rule + .parseRules("[r1: (?x p ?t) <- (?x rdf:type C1), makeInstance(?x, p, C2, ?t)]" + + "[r2: (?t rdf:type C2) <- (?x rdf:type C1), makeInstance(?x, p, C2, ?t)]"); + + FBRuleInfGraph infgraph = (FBRuleInfGraph) createReasoner(rules).bind( + data); + + LPBRuleEngine engine = getEngineForGraph(infgraph); + assertEquals(0, engine.activeInterpreters.size()); + assertEquals(0, engine.tabledGoals.size()); + + // we ask for a non-hit -- it works, but only because we call it.hasNext() + ExtendedIterator<Triple> it = infgraph.find(nohit, ty, C1); + assertFalse(it.hasNext()); + it.close(); + assertEquals(0, engine.activeInterpreters.size()); + + // and again. + // Ensure this is not cached by asking for a different triple pattern + ExtendedIterator<Triple> it2 = infgraph.find(nohit, ty, C2); + // uuups, forgot to call it.hasNext(). But .close() should tidy + it2.close(); + assertEquals(0, engine.activeInterpreters.size()); + + + // OK, let's ask for something that is in the graph + + ExtendedIterator<Triple> it3 = infgraph.find(a, ty, C1); + assertTrue(it3.hasNext()); + assertEquals(a, it3.next().getMatchSubject()); + + // .. and what if we forget to call next() to consume b? + // (e.g. return from a method with the first hit) + + // this should be enough + it3.close(); + // without leaks of activeInterpreters + assertEquals(0, engine.activeInterpreters.size()); + } + + @Test + public void testTabledGoalsCacheHits() throws Exception { + Graph data = Factory.createGraphMem(); + data.add(new Triple(a, ty, C1)); + List<Rule> rules = Rule + .parseRules("[r1: (?x p ?t) <- (?x rdf:type C1), makeInstance(?x, p, C2, ?t)]" + + "[r2: (?t rdf:type C2) <- (?x rdf:type C1), makeInstance(?x, p, C2, ?t)]"); + + FBRuleInfGraph infgraph = (FBRuleInfGraph) createReasoner(rules).bind( + data); + + LPBRuleEngine engine = getEngineForGraph(infgraph); + assertEquals(0, engine.activeInterpreters.size()); + assertEquals(0, engine.tabledGoals.size()); + + ExtendedIterator<Triple> it = infgraph.find(a, ty, C1); + while (it.hasNext()) { + it.next(); + // FIXME: Why do I need to consume all from the iterator + // to avoid leaking activeInterpreters? Calling .close() + // below should have been enough. + } + it.close(); + // how many were cached + assertEquals(1, engine.tabledGoals.size()); + // and no leaks of activeInterpreters + assertEquals(0, engine.activeInterpreters.size()); + + // Now ask again: + it = infgraph.find(a, ty, C1); + while (it.hasNext()) { + it.next(); + } + it.close(); + // if it was a cache hit, no change here: + assertEquals(1, engine.tabledGoals.size()); + assertEquals(0, engine.activeInterpreters.size()); + } + + /** + * Use introspection to get to the LPBRuleEngine. + * <p> + * We are crossing package boundaries and therefore this test would always + * be in the wrong package for either FBRuleInfGraph or LPBRuleEngine. + * <p> + * <strong>This method should only be used for test purposes.</strong> + * + * @param infgraph + * @return + * @throws SecurityException + * @throws NoSuchFieldException + * @throws IllegalArgumentException + * @throws IllegalAccessException + */ + private LPBRuleEngine getEngineForGraph(FBRuleInfGraph infgraph) + throws NoSuchFieldException, SecurityException, + IllegalArgumentException, IllegalAccessException { + Field bEngine = FBRuleInfGraph.class.getDeclaredField("bEngine"); + bEngine.setAccessible(true); + LPBRuleEngine engine = (LPBRuleEngine) bEngine.get(infgraph); + return engine; + } + +} http://git-wip-us.apache.org/repos/asf/jena/blob/4edc6d38/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/test/TestPackage.java ---------------------------------------------------------------------- diff --git a/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/test/TestPackage.java b/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/test/TestPackage.java index af51f6d..1a25ae7 100755 --- a/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/test/TestPackage.java +++ b/jena-core/src/test/java/org/apache/jena/reasoner/rulesys/test/TestPackage.java @@ -20,9 +20,12 @@ package org.apache.jena.reasoner.rulesys.test; import junit.framework.TestSuite ; + import org.slf4j.Logger ; import org.slf4j.LoggerFactory ; +import org.apache.jena.reasoner.rulesys.impl.TestLPBRuleEngineLeak; + /** * Aggregate tester that runs all the test associated with the rulesys package. */ @@ -49,6 +52,7 @@ public class TestPackage extends TestSuite { addTest( "TestGenericRules", TestGenericRules.suite() ); addTest( "TestRETE", TestRETE.suite() ); addTest( TestSetRules.suite() ); + addTest( TestLPBRuleEngineLeak.suite() ); addTest( "OWLRuleUnitTests", OWLUnitTest.suite() ); addTest( "TestBugs", TestBugs.suite() ); addTest( "TestOWLMisc", TestOWLMisc.suite() );
