And another one ... this time involving LPTopGoalIterator.close
I have (in the "simplified branch") remove all method locking in
LPTopGoalIterator and made sure all locks on LPTopGoalIterator are done
either with no nested lock or inside a local synchronized on the
LPBRuleEngine instance.
Many test cycles run, inside Eclispe and in Maven (which show different
behaviours) ... so far, no deadlocks.
Andy
On 02/12/12 21:00, Andy Seaborne wrote:
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
