diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockClient.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockClient.java index dbf72d34ac1c..c3761074fdd7 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockClient.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockClient.java @@ -27,7 +27,6 @@ import org.neo4j.collection.primitive.PrimitiveIntObjectVisitor; import org.neo4j.collection.primitive.PrimitiveLongObjectMap; import org.neo4j.collection.primitive.PrimitiveLongObjectVisitor; -import org.neo4j.helpers.collection.Visitor; import org.neo4j.kernel.impl.locking.LockClientAlreadyClosedException; import org.neo4j.kernel.impl.locking.LockClientStateHolder; import org.neo4j.kernel.impl.locking.Locks; @@ -46,6 +45,10 @@ public class CommunityLockClient implements Locks.Client private final PrimitiveIntObjectMap> sharedLocks = Primitive.intObjectMap(); private final PrimitiveIntObjectMap> exclusiveLocks = Primitive.intObjectMap(); + private final PrimitiveLongObjectVisitor readReleaser; + private final PrimitiveLongObjectVisitor writeReleaser; + private final PrimitiveIntObjectVisitor,RuntimeException> typeReadReleaser; + private final PrimitiveIntObjectVisitor,RuntimeException> typeWriteReleaser; // To be able to close Locks.Client instance properly we should be able to do couple of things: // - have a possibility to prevent new clients to come @@ -58,6 +61,30 @@ public class CommunityLockClient implements Locks.Client public CommunityLockClient( LockManagerImpl manager ) { this.manager = manager; + + readReleaser = ( long key, LockResource lockResource ) -> + { + manager.releaseReadLock( lockResource, lockTransaction ); + return false; + }; + + writeReleaser = ( long key, LockResource lockResource ) -> + { + manager.releaseWriteLock( lockResource, lockTransaction ); + return false; + }; + + typeReadReleaser = ( int key, PrimitiveLongObjectMap value ) -> + { + value.visitEntries( readReleaser ); + return false; + }; + + typeWriteReleaser = ( int key, PrimitiveLongObjectMap value ) -> + { + value.visitEntries( writeReleaser ); + return false; + }; } @Override @@ -94,8 +121,6 @@ public void acquireShared( ResourceType resourceType, long resourceId ) } } - - @Override public void acquireExclusive( ResourceType resourceType, long resourceId ) { @@ -305,14 +330,9 @@ private void releaseLocks() // waking up and terminate all waiters that were waiting for any lock for current client private void terminateAllWaiters() { - manager.accept( new Visitor() - { - @Override - public boolean visit( RWLock lock ) throws RuntimeException - { - lock.terminateLockRequestsForLockTransaction( lockTransaction ); - return false; - } + manager.accept( lock -> { + lock.terminateLockRequestsForLockTransaction( lockTransaction ); + return false; } ); } @@ -325,7 +345,7 @@ public int getLockSessionId() private PrimitiveLongObjectMap localShared( ResourceType resourceType ) { PrimitiveLongObjectMap map = sharedLocks.get( resourceType.typeId() ); - if(map == null) + if ( map == null ) { map = Primitive.longObjectMap(); sharedLocks.put( resourceType.typeId(), map ); @@ -336,7 +356,7 @@ private PrimitiveLongObjectMap localShared( ResourceType resourceT private PrimitiveLongObjectMap localExclusive( ResourceType resourceType ) { PrimitiveLongObjectMap map = exclusiveLocks.get( resourceType.typeId() ); - if(map == null) + if ( map == null ) { map = Primitive.longObjectMap(); exclusiveLocks.put( resourceType.typeId(), map ); @@ -344,48 +364,6 @@ private PrimitiveLongObjectMap localExclusive( ResourceType resour return map; } - private final PrimitiveIntObjectVisitor, RuntimeException> typeReadReleaser = new - PrimitiveIntObjectVisitor, RuntimeException>() - { - @Override - public boolean visited( int key, PrimitiveLongObjectMap value ) throws RuntimeException - { - value.visitEntries( readReleaser ); - return false; - } - }; - - private final PrimitiveIntObjectVisitor, RuntimeException> typeWriteReleaser = new - PrimitiveIntObjectVisitor, RuntimeException>() - { - @Override - public boolean visited( int key, PrimitiveLongObjectMap value ) throws RuntimeException - { - value.visitEntries( writeReleaser ); - return false; - } - }; - - private final PrimitiveLongObjectVisitor writeReleaser = new PrimitiveLongObjectVisitor() - { - @Override - public boolean visited( long key, LockResource lockResource ) throws RuntimeException - { - manager.releaseWriteLock( lockResource, lockTransaction ); - return false; - } - }; - - private final PrimitiveLongObjectVisitor readReleaser = new PrimitiveLongObjectVisitor() - { - @Override - public boolean visited( long key, LockResource lockResource ) throws RuntimeException - { - manager.releaseReadLock( lockResource, lockTransaction ); - return false; - } - }; - @Override public String toString() { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockManger.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockManger.java index abc2cb9c19e2..330b5750aa7b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockManger.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/CommunityLockManger.java @@ -42,20 +42,15 @@ public Client newClient() @Override public void accept( final Visitor visitor ) { - manager.accept( new org.neo4j.helpers.collection.Visitor() - { - @Override - public boolean visit( RWLock element ) throws RuntimeException + manager.accept( element -> { + Object resource = element.resource(); + if ( resource instanceof LockResource ) { - Object resource = element.resource(); - if(resource instanceof LockResource) - { - LockResource lockResource = (LockResource)resource; - visitor.visit( lockResource.type(), lockResource.resourceId(), - element.describe(), element.maxWaitTime(), System.identityHashCode( lockResource ) ); - } - return false; + LockResource lockResource = (LockResource) resource; + visitor.visit( lockResource.type(), lockResource.resourceId(), + element.describe(), element.maxWaitTime(), System.identityHashCode( lockResource ) ); } + return false; } ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockException.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockException.java index d07890a16b2b..dd6639c3b4a3 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockException.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockException.java @@ -25,11 +25,6 @@ */ public class LockException extends RuntimeException { - public LockException() - { - super(); - } - public LockException( String message ) { super( message ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockManagerImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockManagerImpl.java index 1f78f4fd9a4c..489bf5815eb8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockManagerImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockManagerImpl.java @@ -37,43 +37,38 @@ public LockManagerImpl( RagManager ragManager ) this.ragManager = ragManager; } - public long getDetectedDeadlockCount() - { - return ragManager.getDeadlockCount(); - } - public boolean getReadLock( Object resource, Object tx ) - throws DeadlockDetectedException, IllegalResourceException + throws DeadlockDetectedException, IllegalResourceException { return unusedResourceGuard( resource, tx, getRWLockForAcquiring( resource, tx ).acquireReadLock( tx ) ); } public boolean tryReadLock( Object resource, Object tx ) - throws IllegalResourceException + throws IllegalResourceException { return unusedResourceGuard( resource, tx, getRWLockForAcquiring( resource, tx ).tryAcquireReadLock( tx ) ); } public boolean getWriteLock( Object resource, Object tx ) - throws DeadlockDetectedException, IllegalResourceException + throws DeadlockDetectedException, IllegalResourceException { - return unusedResourceGuard(resource, tx, getRWLockForAcquiring( resource, tx ).acquireWriteLock( tx ) ); + return unusedResourceGuard( resource, tx, getRWLockForAcquiring( resource, tx ).acquireWriteLock( tx ) ); } public boolean tryWriteLock( Object resource, Object tx ) - throws IllegalResourceException + throws IllegalResourceException { return unusedResourceGuard( resource, tx, getRWLockForAcquiring( resource, tx ).tryAcquireWriteLock( tx ) ); } public void releaseReadLock( Object resource, Object tx ) - throws LockNotFoundException, IllegalResourceException + throws LockNotFoundException, IllegalResourceException { getRWLockForReleasing( resource, tx, 1, 0, true ).releaseReadLock( tx ); } public void releaseWriteLock( Object resource, Object tx ) - throws LockNotFoundException, IllegalResourceException + throws LockNotFoundException, IllegalResourceException { getRWLockForReleasing( resource, tx, 0, 1, true ).releaseWriteLock( tx ); } @@ -102,8 +97,9 @@ public void dumpLocksOnResource( final Object resource, Logger logger ) * * @return {@code lockObtained } **/ - private boolean unusedResourceGuard(Object resource, Object tx, boolean lockObtained) { - if (!lockObtained) + private boolean unusedResourceGuard( Object resource, Object tx, boolean lockObtained ) + { + if ( !lockObtained ) { // if lock was not acquired cleaning up optimistically allocated value // for case when it was only used by current call, if it was used by somebody else @@ -115,12 +111,12 @@ private boolean unusedResourceGuard(Object resource, Object tx, boolean lockObta /** * Visit all locks. - * + *

* The supplied visitor may not block. * * @param visitor visitor for visiting each lock. */ - public void accept( Visitor visitor ) + public void accept( Visitor visitor ) { synchronized ( resourceLockMap ) { @@ -165,24 +161,25 @@ protected RWLock createLock( Object resource ) } private RWLock getRWLockForReleasing( Object resource, Object tx, int readCountPrerequisite, - int writeCountPrerequisite, boolean strict ) + int writeCountPrerequisite, boolean strict ) { assertValidArguments( resource, tx ); synchronized ( resourceLockMap ) { RWLock lock = resourceLockMap.get( resource ); - if (lock == null ) + if ( lock == null ) { - if (!strict) + if ( !strict ) { return null; } throw new LockNotFoundException( "Lock not found for: " - + resource + " tx:" + tx ); + + resource + " tx:" + tx ); } // we need to get info from a couple of synchronized methods // to make it info consistent we need to synchronized lock to make sure it will not change between // various calls + //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized ( lock ) { if ( !lock.isMarked() && lock.getReadCount() == readCountPrerequisite && diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockResource.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockResource.java index f568b0fbb03f..d5b96f09584f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockResource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/LockResource.java @@ -49,17 +49,8 @@ public boolean equals( Object o ) } LockResource that = (LockResource) o; + return resourceId == that.resourceId && resourceType.equals( that.resourceType ); - if ( resourceId != that.resourceId ) - { - return false; - } - if ( !resourceType.equals( that.resourceType ) ) - { - return false; - } - - return true; } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RWLock.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RWLock.java index 9e4a2cba3a54..90a9a512a3db 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RWLock.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RWLock.java @@ -38,24 +38,24 @@ /** * A read/write lock is a lock that will allow many transactions to acquire read * locks as long as there is no transaction holding the write lock. - *

+ *

* When a transaction has write lock no other tx is allowed to acquire read or * write lock on that resource but the tx holding the write lock. If one tx has * acquired write lock and another tx needs a lock on the same resource that tx * must wait. When the lock is released the other tx is notified and wakes up so * it can acquire the lock. - *

+ *

* Waiting for locks may lead to a deadlock. Consider the following scenario. Tx * T1 acquires write lock on resource R1. T2 acquires write lock on R2. Now T1 * tries to acquire read lock on R2 but has to wait since R2 is locked by T2. If * T2 now tries to acquire a lock on R1 it also has to wait because R1 is locked * by T1. T2 cannot wait on R1 because that would lead to a deadlock where T1 * and T2 waits forever. - *

+ *

* Avoiding deadlocks can be done by keeping a resource allocation graph. This * class works together with the {@link RagManager} to make sure no deadlocks * occur. - *

+ *

* Waiting transactions are put into a queue and when some tx releases the lock * the queue is checked for waiting txs. This implementation tries to avoid lock * starvation and increase performance since only waiting txs that can acquire @@ -65,7 +65,7 @@ class RWLock { private final Object resource; // the resource this RWLock locks private final LinkedList waitingThreadList = new LinkedList<>(); - private final ArrayMap txLockElementMap = new ArrayMap<>( (byte)5, false, true ); + private final ArrayMap txLockElementMap = new ArrayMap<>( (byte) 5, false, true ); private final RagManager ragManager; // access to these is guarded by synchronized blocks @@ -171,16 +171,15 @@ synchronized boolean isMarked() * this.writeCount is greater than the currents tx's write * count the transaction has to wait and the {@link RagManager#checkWaitOn} * method is invoked for deadlock detection. - *

+ *

* If the lock can be acquired the lock count is updated on this * and the transaction lock element (tle). * Waiting for a lock can also be terminated. In that case waiting thread will be interrupted and corresponding * {@link org.neo4j.kernel.impl.locking.community.RWLock.TxLockElement} will be marked as terminated. * In that case lock will not be acquired and false will be return as result of acquisition * - * @throws DeadlockDetectedException - * if a deadlock is detected * @return true is lock was acquired, false otherwise + * @throws DeadlockDetectedException if a deadlock is detected */ synchronized boolean acquireReadLock( Object tx ) throws DeadlockDetectedException { @@ -204,16 +203,7 @@ synchronized boolean acquireReadLock( Object tx ) throws DeadlockDetectedExcepti waitingThreadList.addFirst( lockRequest ); } - try - { - wait(); - addLockRequest = false; - } - catch ( InterruptedException e ) - { - interrupted(); - addLockRequest = true; - } + addLockRequest = waitUninterruptedly(); ragManager.stopWaitOn( this, tx ); } @@ -221,7 +211,9 @@ synchronized boolean acquireReadLock( Object tx ) throws DeadlockDetectedExcepti { registerReadLockAcquired( tx, tle ); return true; - } else { + } + else + { // in case if lock element was interrupted and it was never register before // we need to clean it from lock element map // if it was register before it will be cleaned up during standard lock release call @@ -266,14 +258,14 @@ synchronized boolean tryAcquireReadLock( Object tx ) } /** - * Releases the read lock held by the provided transaction. If it is null then - * an attempt to acquire the current transaction will be made. This is to - * make safe calling the method from the context of an - * afterCompletion() hook where the tx is locally stored and - * not necessarily available through the tm. If there are waiting - * transactions in the queue they will be interrupted if they can acquire - * the lock. - */ + * Releases the read lock held by the provided transaction. If it is null then + * an attempt to acquire the current transaction will be made. This is to + * make safe calling the method from the context of an + * afterCompletion() hook where the tx is locally stored and + * not necessarily available through the tm. If there are waiting + * transactions in the queue they will be interrupted if they can acquire + * the lock. + */ synchronized void releaseReadLock( Object tx ) throws LockNotFoundException { TxLockElement tle = getLockElement( tx ); @@ -356,16 +348,15 @@ else if ( lockRequest.lockType == LockType.READ ) * count or the read count is greater than the currents tx's read count the * transaction has to wait and the {@link RagManager#checkWaitOn} method is * invoked for deadlock detection. - *

+ *

* If the lock can be acquires the lock count is updated on this * and the transaction lock element (tle). * Waiting for a lock can also be terminated. In that case waiting thread will be interrupted and corresponding * {@link org.neo4j.kernel.impl.locking.community.RWLock.TxLockElement} will be marked as terminated. * In that case lock will not be acquired and false will be return as result of acquisition * - * @throws DeadlockDetectedException - * if a deadlock is detected * @return true is lock was acquired, false otherwise + * @throws DeadlockDetectedException if a deadlock is detected */ synchronized boolean acquireWriteLock( Object tx ) throws DeadlockDetectedException { @@ -390,16 +381,7 @@ synchronized boolean acquireWriteLock( Object tx ) throws DeadlockDetectedExcept waitingThreadList.addFirst( lockRequest ); } - try - { - wait(); - addLockRequest = false; - } - catch ( InterruptedException e ) - { - interrupted(); - addLockRequest = true; - } + addLockRequest = waitUninterruptedly(); ragManager.stopWaitOn( this, tx ); } @@ -407,7 +389,9 @@ synchronized boolean acquireWriteLock( Object tx ) throws DeadlockDetectedExcept { registerWriteLockAcquired( tx, tle ); return true; - } else { + } + else + { // in case if lock element was interrupted and it was never register before // we need to clean it from lock element map // if it was register before it will be cleaned up during standard lock release call @@ -430,10 +414,26 @@ synchronized boolean acquireWriteLock( Object tx ) throws DeadlockDetectedExcept } } + private boolean waitUninterruptedly() + { + boolean addLockRequest; + try + { + wait(); + addLockRequest = false; + } + catch ( InterruptedException e ) + { + interrupted(); + addLockRequest = true; + } + return addLockRequest; + } + // in case of spurious wake up, deadlock during spurious wake up, termination // when we already have request in a queue we need to clean it up private void cleanupWaitingListRequests( LockRequest lockRequest, TxLockElement lockElement, - boolean addLockRequest ) + boolean addLockRequest ) { if ( lockRequest != null && (lockElement.isTerminated() || !addLockRequest) ) { @@ -463,14 +463,14 @@ synchronized boolean tryAcquireWriteLock( Object tx ) } /** - * Releases the write lock held by the provided tx. If it is null then an - * attempt to acquire the current transaction from the transaction manager - * will be made. This is to make safe calling this method as an - * afterCompletion() hook where the transaction context is not - * necessarily available. If write count is zero and there are waiting - * transactions in the queue they will be interrupted if they can acquire - * the lock. - */ + * Releases the write lock held by the provided tx. If it is null then an + * attempt to acquire the current transaction from the transaction manager + * will be made. This is to make safe calling this method as an + * afterCompletion() hook where the transaction context is not + * necessarily available. If write count is zero and there are waiting + * transactions in the queue they will be interrupted if they can acquire + * the lock. + */ synchronized void releaseWriteLock( Object tx ) throws LockNotFoundException { TxLockElement tle = getLockElement( tx ); @@ -531,7 +531,7 @@ synchronized int getWaitingThreadsCount() public synchronized boolean logTo( Logger logger ) { logger.log( "Total lock count: readCount=" + totalReadCount - + " writeCount=" + totalWriteCount + " for " + resource ); + + " writeCount=" + totalWriteCount + " for " + resource ); logger.log( "Waiting list:" ); Iterator wElements = waitingThreadList.iterator(); @@ -539,8 +539,8 @@ public synchronized boolean logTo( Logger logger ) { LockRequest lockRequest = wElements.next(); logger.log( "[" + lockRequest.waitingThread + "(" - + lockRequest.element.readCount + "r," + lockRequest.element.writeCount + "w)," - + lockRequest.lockType + "]" ); + + lockRequest.element.readCount + "r," + lockRequest.element.writeCount + "w)," + + lockRequest.lockType + "]" ); if ( wElements.hasNext() ) { logger.log( "," ); @@ -552,12 +552,10 @@ public synchronized boolean logTo( Logger logger ) } logger.log( "Locking transactions:" ); - Iterator lElements = txLockElementMap.values().iterator(); - while ( lElements.hasNext() ) + for ( TxLockElement tle : txLockElementMap.values() ) { - TxLockElement tle = lElements.next(); logger.log( "" + tle.tx + "(" + tle.readCount + "r," - + tle.writeCount + "w)" ); + + tle.writeCount + "w)" ); } return true; } @@ -565,16 +563,16 @@ public synchronized boolean logTo( Logger logger ) public synchronized String describe() { StringBuilder sb = new StringBuilder( this.toString() ); - sb.append( " Total lock count: readCount=" + totalReadCount - + " writeCount=" + totalWriteCount + " for " + resource + "\n" ) + sb.append( " Total lock count: readCount=" ).append( totalReadCount ).append( " writeCount=" ) + .append( totalWriteCount ).append( " for " ).append( resource ).append( "\n" ) .append( "Waiting list:" + "\n" ); Iterator wElements = waitingThreadList.iterator(); while ( wElements.hasNext() ) { LockRequest lockRequest = wElements.next(); - sb.append( "[" + lockRequest.waitingThread + "(" - + lockRequest.element.readCount + "r," + lockRequest.element.writeCount + "w)," - + lockRequest.lockType + "]\n" ); + sb.append( "[" ).append( lockRequest.waitingThread ).append( "(" ).append( lockRequest.element.readCount ) + .append( "r," ).append( lockRequest.element.writeCount ).append( "w)," ).append( lockRequest.lockType ) + .append( "]\n" ); if ( wElements.hasNext() ) { sb.append( "," ); @@ -582,12 +580,10 @@ public synchronized String describe() } sb.append( "Locking transactions:\n" ); - Iterator lElements = txLockElementMap.values().iterator(); - while ( lElements.hasNext() ) + for ( TxLockElement tle : txLockElementMap.values() ) { - TxLockElement tle = lElements.next(); - sb.append( "" + tle.tx + "(" + tle.readCount + "r," - + tle.writeCount + "w)\n" ); + sb.append( "" ).append( tle.tx ).append( "(" ).append( tle.readCount ).append( "r," ) + .append( tle.writeCount ).append( "w)\n" ); } return sb.toString(); } @@ -607,9 +603,10 @@ public synchronized long maxWaitTime() // for specified transaction object mark all lock elements as terminated // and interrupt all waiters - synchronized void terminateLockRequestsForLockTransaction(Object lockTransaction) { + synchronized void terminateLockRequestsForLockTransaction( Object lockTransaction ) + { TxLockElement lockElement = txLockElementMap.get( lockTransaction ); - if ( lockElement != null && !lockElement.isTerminated()) + if ( lockElement != null && !lockElement.isTerminated() ) { lockElement.setTerminated( true ); for ( LockRequest lockRequest : waitingThreadList ) @@ -625,7 +622,7 @@ synchronized void terminateLockRequestsForLockTransaction(Object lockTransaction @Override public String toString() { - return "RWLock[" + resource + ", hash="+hashCode()+"]"; + return "RWLock[" + resource + ", hash=" + hashCode() + "]"; } private void registerReadLockAcquired( Object tx, TxLockElement tle ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RagManager.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RagManager.java index 7e12342d75e0..73ed5f27b807 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RagManager.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/community/RagManager.java @@ -25,7 +25,6 @@ import java.util.List; import java.util.Map; import java.util.Stack; -import java.util.concurrent.atomic.AtomicInteger; import org.neo4j.kernel.DeadlockDetectedException; import org.neo4j.kernel.impl.util.ArrayMap; @@ -37,11 +36,11 @@ * wait and that may lead to a deadlock. So before the tx is put into wait mode * the {@link RagManager#checkWaitOn} method is invoked to check if a wait of * this transaction will lead to a deadlock. - *

+ *

* The checkWaitOn throws a {@link DeadlockDetectedException} if * a deadlock would occur when the transaction would wait for the resource. That * will guarantee that a deadlock never occurs on a RWLock basis. - *

+ *

* Think of the resource allocation graph as a graph. We have two node * types, resource nodes (R) and tx/process nodes (T). When a transaction * acquires lock on some resource a relationship is added from the resource to @@ -71,14 +70,7 @@ public class RagManager private final Map> resourceMap = new HashMap<>(); private final ArrayMap waitingTxMap = - new ArrayMap<>( (byte)5, false, true ); - - private final AtomicInteger deadlockCount = new AtomicInteger(); - - long getDeadlockCount() - { - return deadlockCount.longValue(); - } + new ArrayMap<>( (byte) 5, false, true ); synchronized void lockAcquired( Object resource, Object tx ) { @@ -124,13 +116,13 @@ synchronized void stopWaitOn( Object resource, Object tx ) // after invoke the transaction must wait on the resource synchronized void checkWaitOn( Object resource, Object tx ) - throws DeadlockDetectedException + throws DeadlockDetectedException { List lockingTxList = resourceMap.get( resource ); if ( lockingTxList == null ) { throw new LockException( "Illegal resource[" + resource - + "], not found in map" ); + + "], not found in map" ); } if ( waitingTxMap.get( tx ) != null ) @@ -171,7 +163,7 @@ synchronized void checkWaitOn( Object resource, Object tx ) } graphStack.push( lockingTx ); checkWaitOnRecursive( lockingTx, tx, checkedTransactions, - graphStack ); + graphStack ); graphStack.pop(); } @@ -180,8 +172,8 @@ synchronized void checkWaitOn( Object resource, Object tx ) } private synchronized void checkWaitOnRecursive( Object lockingTx, - Object waitingTx, List checkedTransactions, - Stack graphStack ) throws DeadlockDetectedException + Object waitingTx, List checkedTransactions, + Stack graphStack ) throws DeadlockDetectedException { if ( lockingTx.equals( waitingTx ) ) { @@ -198,13 +190,13 @@ private synchronized void checkWaitOnRecursive( Object lockingTx, } else { - circle.append( " <-[:WAITING_FOR]- " ).append( lockingTx ).append( " <-[:HELD_BY]- " ).append( resource ); + circle.append( " <-[:WAITING_FOR]- " ).append( lockingTx ).append( " <-[:HELD_BY]- " ) + .append( resource ); } } while ( !graphStack.isEmpty() ); - deadlockCount.incrementAndGet(); - throw new DeadlockDetectedException( waitingTx + - " can't wait on resource " + resource + " since => " + circle ); + throw new DeadlockDetectedException( + waitingTx + " can't wait on resource " + resource + " since => " + circle ); } checkedTransactions.add( lockingTx ); Object resource = waitingTxMap.get( lockingTx ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/community/RWLockTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/community/RWLockTest.java index db8e6e73bf0f..999a5c0ee589 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/community/RWLockTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/locking/community/RWLockTest.java @@ -24,8 +24,6 @@ import org.junit.BeforeClass; import org.junit.Test; import org.mockito.Mockito; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; @@ -100,12 +98,14 @@ public void assertReadLockDoesNotLeakMemory() throws InterruptedException public void testWaitingWriterLock() throws InterruptedException { RagManager ragManager = new RagManager(); - LockResource resource = new LockResource( ResourceTypes.NODE, 1l ); + LockResource resource = new LockResource( ResourceTypes.NODE, 1L ); final RWLock lock = new RWLock( resource, ragManager ); final LockTransaction lockTransaction = new LockTransaction(); final LockTransaction anotherTransaction = new LockTransaction(); + lock.mark(); lock.acquireReadLock( lockTransaction ); + lock.mark(); lock.acquireReadLock( anotherTransaction ); final CountDownLatch writerCompletedLatch = new CountDownLatch( 1 ); @@ -143,13 +143,14 @@ public void testWaitingWriterLock() throws InterruptedException public void testWaitingReaderLock() throws InterruptedException { RagManager ragManager = new RagManager(); - LockResource resource = new LockResource( ResourceTypes.NODE, 1l ); + LockResource resource = new LockResource( ResourceTypes.NODE, 1L ); final RWLock lock = new RWLock( resource, ragManager ); final LockTransaction transaction = new LockTransaction(); final LockTransaction readerTransaction = new LockTransaction(); final CountDownLatch readerCompletedLatch = new CountDownLatch( 1 ); + lock.mark(); lock.acquireWriteLock( transaction ); Runnable reader = createReader( lock, readerTransaction, readerCompletedLatch ); @@ -181,7 +182,7 @@ public void testWaitingReaderLock() throws InterruptedException public void testThreadRemovedFromWaitingListOnDeadlock() throws InterruptedException { RagManager ragManager = Mockito.mock( RagManager.class ); - LockResource resource = new LockResource( ResourceTypes.NODE, 1l ); + LockResource resource = new LockResource( ResourceTypes.NODE, 1L ); final RWLock lock = new RWLock( resource, ragManager ); final LockTransaction lockTransaction = new LockTransaction(); final LockTransaction anotherTransaction = new LockTransaction(); @@ -189,37 +190,30 @@ public void testThreadRemovedFromWaitingListOnDeadlock() throws InterruptedExcep final CountDownLatch exceptionLatch = new CountDownLatch( 1 ); final CountDownLatch completionLatch = new CountDownLatch( 1 ); - Mockito.doNothing().doAnswer( new Answer() - { - @Override - public Void answer( InvocationOnMock invocation ) throws Throwable - { - exceptionLatch.countDown(); - throw new DeadlockDetectedException( "Deadlock" ); - } + Mockito.doNothing().doAnswer( invocation -> { + exceptionLatch.countDown(); + throw new DeadlockDetectedException( "Deadlock" ); } ).when( ragManager ).checkWaitOn( lock, lockTransaction ); + lock.mark(); + lock.mark(); lock.acquireReadLock( lockTransaction ); lock.acquireReadLock( anotherTransaction ); // writer will be added to a waiting list // then spurious wake up will be simulated // and deadlock will be detected - Runnable writer = new Runnable() - { - @Override - public void run() + Runnable writer = () -> { + try { - try - { - lock.acquireWriteLock( lockTransaction ); - } - catch ( DeadlockDetectedException ignored ) - { - // ignored - } - completionLatch.countDown(); + lock.mark(); + lock.acquireWriteLock( lockTransaction ); } + catch ( DeadlockDetectedException ignored ) + { + // ignored + } + completionLatch.countDown(); }; executor.execute( writer ); @@ -228,6 +222,7 @@ public void run() // sending notify for all threads till our writer will not cause deadlock exception do { + //noinspection SynchronizationOnLocalVariableOrMethodParameter synchronized ( lock ) { lock.notifyAll(); @@ -239,14 +234,14 @@ public void run() completionLatch.await(); assertEquals( "In case of deadlock caused by spurious wake up " + - "thread should be removed from waiting list", 0, lock.getWaitingThreadsCount() ); + "thread should be removed from waiting list", 0, lock.getWaitingThreadsCount() ); } @Test public void testLockCounters() throws InterruptedException { RagManager ragManager = new RagManager(); - LockResource resource = new LockResource( ResourceTypes.NODE, 1l ); + LockResource resource = new LockResource( ResourceTypes.NODE, 1L ); final RWLock lock = new RWLock( resource, ragManager ); LockTransaction lockTransaction = new LockTransaction(); LockTransaction anotherTransaction = new LockTransaction(); @@ -254,7 +249,9 @@ public void testLockCounters() throws InterruptedException final CountDownLatch writerCompletedLatch = new CountDownLatch( 1 ); + lock.mark(); lock.acquireReadLock( lockTransaction ); + lock.mark(); lock.acquireReadLock( anotherTransaction ); assertEquals( 2, lock.getReadCount() ); @@ -296,9 +293,9 @@ public void testLockCounters() throws InterruptedException public void testDeadlockDetection() throws InterruptedException { RagManager ragManager = new RagManager(); - LockResource node1 = new LockResource( ResourceTypes.NODE, 1l ); - LockResource node2 = new LockResource( ResourceTypes.NODE, 2l ); - LockResource node3 = new LockResource( ResourceTypes.NODE, 3l ); + LockResource node1 = new LockResource( ResourceTypes.NODE, 1L ); + LockResource node2 = new LockResource( ResourceTypes.NODE, 2L ); + LockResource node3 = new LockResource( ResourceTypes.NODE, 3L ); final RWLock lockNode1 = new RWLock( node1, ragManager ); final RWLock lockNode2 = new RWLock( node2, ragManager ); @@ -310,8 +307,11 @@ public void testDeadlockDetection() throws InterruptedException final CountDownLatch deadLockDetector = new CountDownLatch( 1 ); + lockNode1.mark(); lockNode1.acquireWriteLock( client1Transaction ); + lockNode2.mark(); lockNode2.acquireWriteLock( client2Transaction ); + lockNode3.mark(); lockNode3.acquireWriteLock( client3Transaction ); Runnable readerLockNode2 = createReaderForDeadlock( lockNode3, client1Transaction, deadLockDetector ); @@ -322,7 +322,8 @@ public void testDeadlockDetection() throws InterruptedException executor.execute( readerLockNode1 ); // Deadlock should occur - Assert.assertTrue( "Deadlock was detected as expected.", deadLockDetector.await( 1000, TimeUnit.MILLISECONDS ) ); + Assert.assertTrue( "Deadlock was detected as expected.", + deadLockDetector.await( 1000, TimeUnit.MILLISECONDS ) ); } @Test( timeout = 1000 ) @@ -330,7 +331,7 @@ public void testLockRequestsTermination() throws InterruptedException { // given RagManager ragManager = new RagManager(); - LockResource node1 = new LockResource( ResourceTypes.NODE, 1l ); + LockResource node1 = new LockResource( ResourceTypes.NODE, 1L ); final RWLock lock = new RWLock( node1, ragManager ); final LockTransaction mainTransaction = new LockTransaction(); @@ -343,6 +344,7 @@ public void testLockRequestsTermination() throws InterruptedException Runnable reader = createFailedReader( lock, readerTransaction, readerCompletedLatch ); // when + lock.mark(); assertTrue( lock.acquireWriteLock( mainTransaction ) ); executor.submit( reader ); executor.submit( conflictingWriter ); @@ -369,77 +371,57 @@ public void testLockRequestsTermination() throws InterruptedException } private Runnable createReader( final RWLock lock, final LockTransaction transaction, - final CountDownLatch latch ) + final CountDownLatch latch ) { - return new Runnable() - { - @Override - public void run() - { - lock.acquireReadLock( transaction ); - latch.countDown(); - } + return () -> { + lock.mark(); + lock.acquireReadLock( transaction ); + latch.countDown(); }; } private Runnable createFailedReader( final RWLock lock, final LockTransaction transaction, - final CountDownLatch latch ) + final CountDownLatch latch ) { - return new Runnable() - { - @Override - public void run() - { - Assert.assertFalse( lock.acquireReadLock( transaction ) ); - latch.countDown(); - } + return () -> { + lock.mark(); + Assert.assertFalse( lock.acquireReadLock( transaction ) ); + latch.countDown(); }; } private Runnable createWriter( final RWLock lock, final LockTransaction transaction, - final CountDownLatch latch ) + final CountDownLatch latch ) { - return new Runnable() - { - @Override - public void run() - { - lock.acquireWriteLock( transaction ); - latch.countDown(); - } + return () -> { + lock.mark(); + lock.acquireWriteLock( transaction ); + latch.countDown(); }; } private Runnable createFailedWriter( final RWLock lock, final LockTransaction transaction, - final CountDownLatch latch ) + final CountDownLatch latch ) { - return new Runnable() - { - @Override - public void run() - { - Assert.assertFalse( lock.acquireWriteLock( transaction ) ); - latch.countDown(); - } + return () -> { + lock.mark(); + Assert.assertFalse( lock.acquireWriteLock( transaction ) ); + latch.countDown(); }; } private Runnable createReaderForDeadlock( final RWLock node, final LockTransaction transaction, - final CountDownLatch latch ) + final CountDownLatch latch ) { - return new Runnable() - { - @Override - public void run() + return () -> { + try + { + node.mark(); + node.acquireReadLock( transaction ); + } + catch ( DeadlockDetectedException e ) { - try - { - node.acquireReadLock( transaction ); - } - catch ( DeadlockDetectedException e ) - { - latch.countDown(); - } + latch.countDown(); } }; }