Re: Concurrency test failure (reasoners)

[Andy Seaborne]

On 02/12/12 19:58, Andy Seaborne wrote:
In the jena-core-simplified branch, I am getting a test failure for
com.hp.hpl.jena.reasoner.rulesys.test.ConcuerrencyTest.testWithOWLMemMicroRuleInfModel


The deadlock does not seem to be from the changes as this looks like an
embrace situation:

A/ In LPTopGoalIterator moveForward is synchronized and then grabs a
lock on an LPBRuleEngine.

So the locking is:
LPTopGoalIterator -> LPBRuleEngine

B/ LPBRuleEngine.checkSafeToUpdate has "Should be called from within a
synchronized block." in the javadoc and it calls
LPTopGoalIterator.close, a synchronized method.

The outer synchronized is LPBRuleEngine.deleteAllRules

so the lock order is

LPBRuleEngine -> LPTopGoalIterator


Oops.

Is the sequence below safe?  Move the synchronized in to cover only the
close and get of the engine.

LPTopGoalIterator --

  private /*synchronized*/ void moveForward() {
         LPBRuleEngine lpEngine ;
         synchronized(this)
         {
             checkClosed();
             lpEngine = interpreter.getEngine();
         }
         synchronized (lpEngine) {
             // Check again, is lpEngine valid?
             checkClosed();

     Andy


That didn't work - I'm seeing other concurrency failures now.

To duplicate the lock ordering in LPBRuleEngine.checkSafeToUpdate I have 
put in (jena-core-simplied branch):

private void moveForward() {
    LPBRuleEngine lpEngine ;
    synchronized(this)
    {
        checkClosed();
        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)
        {
            checkClosed();
         ....

which, so far, has been passing.

        Andy