You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by an...@apache.org on 2012/12/07 10:41:53 UTC

svn commit: r1418235 - /jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java

Author: andy
Date: Fri Dec  7 09:41:52 2012
New Revision: 1418235

URL: http://svn.apache.org/viewvc?rev=1418235&view=rev
Log:
Fix for JENA-244 integrated into trunk for testing.

Modified:
    jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java

Modified: jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java?rev=1418235&r1=1418234&r2=1418235&view=diff
==============================================================================
--- jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java (original)
+++ jena/trunk/jena-core/src/main/java/com/hp/hpl/jena/reasoner/rulesys/impl/LPTopGoalIterator.java Fri Dec  7 09:41:52 2012
@@ -73,34 +73,50 @@ public class LPTopGoalIterator implement
         engine.setTopInterpreter(this);
     }
 
+    /* A Note on lock ordering:
+     * 
+     * Elsewhere code takes an LPBRuleEngine then an LPTopGoalIterator
+     * Ensure we do that lock order here as well as just synchronized 
+     * on the method reverses the lock ordering, leading to deadlock.
+     */
+    
+    
     /**
      * Find the next result in the goal state and put it in the
      * lookahead buffer.
      */
-    private synchronized void moveForward() {
-        checkClosed();
-        LPBRuleEngine lpEngine = interpreter.getEngine();
+    private void moveForward() {
+        LPBRuleEngine lpEngine ;
+        synchronized(this)
+        {
+            checkClosed();
+            lpEngine = interpreter.getEngine();
+        }
         synchronized (lpEngine) {
-
-            lookaheadValid = true;
-
-            // TODO nasty dynamic typing here.
-            Object next = interpreter.next();
-            lookAhead = next instanceof Triple ? (Triple) next : null;
-            if (next == StateFlag.FAIL) {
-                if (choicePoints.isEmpty()) {
-                    // Nothing left to try
-                    close();
-                } else {
-                    // Some options open, continue pumping
-                    nextToRun = null;
-                    lpEngine.pump(this);
-                    if (nextToRun == null) {
-                        // Reached final closure
+            synchronized(this)
+            {
+                checkClosed();
+
+                lookaheadValid = true;
+
+                // TODO nasty dynamic typing here.
+                Object next = interpreter.next();
+                lookAhead = next instanceof Triple ? (Triple) next : null;
+                if (next == StateFlag.FAIL) {
+                    if (choicePoints.isEmpty()) {
+                        // Nothing left to try
                         close();
                     } else {
-                        interpreter.setState(nextToRun);
-                        moveForward();
+                        // Some options open, continue pumping
+                        nextToRun = null;
+                        lpEngine.pump(this);
+                        if (nextToRun == null) {
+                            // Reached final closure
+                            close();
+                        } else {
+                            interpreter.setState(nextToRun);
+                            moveForward();
+                        }
                     }
                 }
             }
@@ -165,24 +181,37 @@ public class LPTopGoalIterator implement
      * @see com.hp.hpl.jena.util.iterator.ClosableIterator#close()
      */
     @Override
-    public synchronized void close() {
-        if (interpreter != null) {
-            synchronized (interpreter.getEngine()) {
-                // LogFactory.getLog( getClass() ).debug( "Entering close sync block on " + interpreter.getEngine() );
-
-                // Check for any dangling generators which are complete
-                interpreter.getEngine().checkForCompletions();
-                // Close this top goal
-                lookAhead = null;
-                //LogFactory.getLog( getClass() ).debug( "Nulling and closing LPTopGoalIterator " + interpreter );
-                interpreter.close();
-                // was TEMP experiment: interpreter.getEngine().detach(interpreter);
-                interpreter = null;
-                isReady = false;
-                checkReadyNeeded = false;
-                nextToRun = null;
-//                choicePoints = null;  // disabled to prevent async close causing problems
-                //LogFactory.getLog( getClass() ).debug( "Leaving close sync block " );
+    public void close() {
+        LPBRuleEngine lpEngine ;
+        synchronized(this)
+        {
+            if ( interpreter == null ) return ;
+            lpEngine = interpreter.getEngine();
+        }
+
+        synchronized (lpEngine) {
+            // Elsewhere code takes an LPBRuleEngine then an LPTopGoalIterator
+            // Ensure we do that lock order here as well as just synchronized 
+            // on the method reverses the locks takne, leading to deadlock.
+            synchronized(this)
+            {
+                if (interpreter != null) {
+                    // LogFactory.getLog( getClass() ).debug( "Entering close sync block on " + interpreter.getEngine() );
+
+                    // Check for any dangling generators which are complete
+                    interpreter.getEngine().checkForCompletions();
+                    // Close this top goal
+                    lookAhead = null;
+                    //LogFactory.getLog( getClass() ).debug( "Nulling and closing LPTopGoalIterator " + interpreter );
+                    interpreter.close();
+                    // was TEMP experiment: interpreter.getEngine().detach(interpreter);
+                    interpreter = null;
+                    isReady = false;
+                    checkReadyNeeded = false;
+                    nextToRun = null;
+                    //                choicePoints = null;  // disabled to prevent async close causing problems
+                    //LogFactory.getLog( getClass() ).debug( "Leaving close sync block " );
+                }
             }
         }
     }