From 26a88d220cc63e5c084bff43781dc8b1aaabf12f Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 8 Mar 2016 10:45:27 +0100 Subject: [PATCH] Use exact maths in the CommunityLockManager --- .../main/java/org/neo4j/helpers/MathUtil.java | 13 +++++++ .../impl/locking/LockClientStateHolder.java | 13 +++++-- .../impl/locking/community/LockResource.java | 5 ++- .../kernel/impl/locking/community/RWLock.java | 39 +++++++++++-------- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/helpers/MathUtil.java b/community/kernel/src/main/java/org/neo4j/helpers/MathUtil.java index d059320f6c19d..838f1a458794e 100644 --- a/community/kernel/src/main/java/org/neo4j/helpers/MathUtil.java +++ b/community/kernel/src/main/java/org/neo4j/helpers/MathUtil.java @@ -89,5 +89,18 @@ public static int compareLongAgainstDouble( long lhs, double rhs ) { return - compareDoubleAgainstLong( rhs, lhs ); } + + /** + * Return an integer one less than the given integer, or throw {@link ArithmeticException} if the given integer is + * zero. + */ + public static int decrementExactNotPastZero( int value ) + { + if ( value == 0 ) + { + throw new ArithmeticException( "integer underflow past zero" ); + } + return value - 1; + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStateHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStateHolder.java index 7fe18293f0e4b..fd5a3dc818407 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStateHolder.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/locking/LockClientStateHolder.java @@ -81,7 +81,7 @@ public boolean incrementActiveClients() return false; } } - while ( !clientState.compareAndSet( currentState, statusWithUpdatedClients( currentState, 1 ) ) ); + while ( !clientState.compareAndSet( currentState, incrementActiveClients( currentState ) ) ); return true; } @@ -95,7 +95,7 @@ public void decrementActiveClients() { currentState = clientState.get(); } - while ( !clientState.compareAndSet( currentState, statusWithUpdatedClients( currentState, -1 ) ) ); + while ( !clientState.compareAndSet( currentState, decrementActiveClients( currentState ) ) ); } /** @@ -136,8 +136,13 @@ private int stateWithNewStatus( int clientState, int newStatus ) return newStatus | getActiveClients( clientState ); } - private int statusWithUpdatedClients( int clientState, int delta ) + private int incrementActiveClients( int clientState ) { - return getStatus( clientState ) | Math.addExact( getActiveClients( clientState ), delta ); + return getStatus( clientState ) | Math.incrementExact( getActiveClients( clientState ) ); + } + + private int decrementActiveClients( int clientState ) + { + return getStatus( clientState ) | Math.decrementExact( getActiveClients( clientState ) ); } } 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 be84c8d564300..f568b0fbb03f7 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 @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.locking.community; +import org.neo4j.helpers.MathUtil; import org.neo4j.storageengine.api.lock.ResourceType; public class LockResource @@ -77,12 +78,12 @@ public String toString() public void acquireReference() { - refCount++; + refCount = Math.incrementExact( refCount ); } public int releaseReference() { - return --refCount; + return refCount = MathUtil.decrementExactNotPastZero( refCount ); } public long resourceId() 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 83ec741843ac0..9e4a2cba3a545 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 @@ -23,6 +23,7 @@ import java.util.LinkedList; import java.util.ListIterator; +import org.neo4j.helpers.MathUtil; import org.neo4j.kernel.DeadlockDetectedException; import org.neo4j.kernel.impl.locking.LockType; import org.neo4j.kernel.impl.util.ArrayMap; @@ -99,12 +100,12 @@ private static class TxLockElement void incrementRequests() { - requests++; + requests = Math.incrementExact( requests ); } void decrementRequests() { - requests--; + requests = MathUtil.decrementExactNotPastZero( requests ); } boolean hasNoRequests() @@ -151,7 +152,13 @@ public Object resource() synchronized void mark() { - this.marked++; + marked = Math.incrementExact( marked ); + } + + /** synchronized by all caller methods */ + private void unmark() + { + marked = MathUtil.decrementExactNotPastZero( marked ); } synchronized boolean isMarked() @@ -233,7 +240,7 @@ synchronized boolean acquireReadLock( Object tx ) throws DeadlockDetectedExcepti interrupted(); // if deadlocked, remove marking so lock is removed when empty tle.decrementRequests(); - marked--; + unmark(); } } @@ -254,7 +261,7 @@ synchronized boolean tryAcquireReadLock( Object tx ) finally { // if deadlocked, remove marking so lock is removed when empty - marked--; + unmark(); } } @@ -276,8 +283,8 @@ synchronized void releaseReadLock( Object tx ) throws LockNotFoundException throw new LockNotFoundException( "" + tx + " don't have readLock" ); } - totalReadCount--; - tle.readCount--; + totalReadCount = MathUtil.decrementExactNotPastZero( totalReadCount ); + tle.readCount = MathUtil.decrementExactNotPastZero( tle.readCount ); if ( tle.isFree() ) { ragManager.lockReleased( this, tx ); @@ -419,7 +426,7 @@ synchronized boolean acquireWriteLock( Object tx ) throws DeadlockDetectedExcept interrupted(); // if deadlocked, remove marking so lock is removed when empty tle.decrementRequests(); - marked--; + unmark(); } } @@ -451,7 +458,7 @@ synchronized boolean tryAcquireWriteLock( Object tx ) finally { // if deadlocked, remove marking so lock is removed when empty - marked--; + unmark(); } } @@ -473,8 +480,8 @@ synchronized void releaseWriteLock( Object tx ) throws LockNotFoundException throw new LockNotFoundException( "" + tx + " don't have writeLock" ); } - totalWriteCount--; - tle.writeCount--; + totalWriteCount = MathUtil.decrementExactNotPastZero( totalWriteCount ); + tle.writeCount = MathUtil.decrementExactNotPastZero( tle.writeCount ); if ( tle.isFree() ) { ragManager.lockReleased( this, tx ); @@ -587,7 +594,7 @@ public synchronized String describe() public synchronized long maxWaitTime() { - long max = 0l; + long max = 0L; for ( LockRequest thread : waitingThreadList ) { if ( thread.since < max ) @@ -624,15 +631,15 @@ public String toString() private void registerReadLockAcquired( Object tx, TxLockElement tle ) { registerLockAcquired( tx, tle ); - totalReadCount++; - tle.readCount++; + totalReadCount = Math.incrementExact( totalReadCount ); + tle.readCount = Math.incrementExact( tle.readCount ); } private void registerWriteLockAcquired( Object tx, TxLockElement tle ) { registerLockAcquired( tx, tle ); - totalWriteCount++; - tle.writeCount++; + totalWriteCount = Math.incrementExact( totalWriteCount ); + tle.writeCount = Math.incrementExact( tle.writeCount ); } private void registerLockAcquired( Object tx, TxLockElement tle )