Skip to content

Commit

Permalink
Missing write and read lock requests in community RWLock
Browse files Browse the repository at this point in the history
When last active resource owner release lock and detect waiting thread (reader or writer)
it removes LockElement from a lock, and interrupts waiting thread.
That will cause totalReadCount or totalWriteCount to be out of sync with LockElement counters (because of LockElement was removed) as soon as waiting thread will take a resource.

This commit should fix a problem when missing write/read lock for a popular resource cause all threads to wait for a resource to be released. Test cases for missing write and read cases provided

Introduce graceful shutdown to community client, now we will wait for all active clients to complete their current requests before closing to prevent a race between closing and locks acquisition
  • Loading branch information
MishaDemianenko committed Aug 3, 2015
1 parent 3ae4c7e commit cdfce78
Show file tree
Hide file tree
Showing 12 changed files with 1,442 additions and 224 deletions.
@@ -0,0 +1,32 @@
/*
* Copyright (c) 2002-2015 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.locking;

/**
* Exception that will be thrown in case when closed {@link org.neo4j.kernel.impl.locking.Locks.Client}
* will be used to acquire shared/exclusive lock
*/
public class LockClientAlreadyClosedException extends RuntimeException
{
public LockClientAlreadyClosedException( String message )
{
super( message );
}
}
@@ -0,0 +1,134 @@
/*
* Copyright (c) 2002-2015 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.locking;

import java.util.concurrent.atomic.AtomicInteger;

/**
* State control class for Locks.Clients.
* Client state represent current Locks.Client state: OPEN/CLOSED
* and number of active clients
*/
public final class LockClientStateHolder
{
private static final int FLAG_BITS = 1;
private static final int CLIENT_BITS = Integer.SIZE - FLAG_BITS;
private static final int STATE_BIT_MASK = 1 << CLIENT_BITS;
private static final int CLOSED = 1 << CLIENT_BITS;
private static final int INITIAL_STATE = 0;
private AtomicInteger clientState = new AtomicInteger( INITIAL_STATE );

/**
* Check if we still have any active client
* @return true if have any open client, false otherwise.
*/
public boolean hasActiveClients()
{
return getActiveClients( clientState.get() ) > 0;
}

/**
* Closing current client
*/
public void closeClient()
{
int currentValue;
do
{
currentValue = clientState.get();
}
while ( !clientState.compareAndSet( currentValue, stateWithNewStatus( currentValue, CLOSED ) ) );
}

/**
* Increment active number of clients that use current state instance.
* @return false if already closed and not possible to increment active clients counter, true in case if counter
* was successfully incremented.
*/
public boolean incrementActiveClients()
{
int currentState;
do
{
currentState = clientState.get();
if ( isClosed( currentState ) )
{
return false;
}
}
while ( !clientState.compareAndSet( currentState, statusWithUpdatedClients( currentState, 1 ) ) );
return true;
}

/**
* Decrement number of active clients that use current client state object.
*/
public void decrementActiveClients()
{
int currentState;
do
{
currentState = clientState.get();
}
while ( !clientState.compareAndSet( currentState, statusWithUpdatedClients( currentState, -1 ) ) );
}

/**
* Check if closed
* @return true if client is closed, false otherwise
*/
public boolean isClosed()
{
return isClosed( clientState.get() );
}

/**
* Reset state to initial state disregard any current state or number of active clients
*/
public void reset()
{
clientState.set( INITIAL_STATE );
}

private boolean isClosed( int clientState )
{
return getStatus( clientState ) == CLOSED;
}

private int getStatus( int clientState )
{
return clientState & STATE_BIT_MASK;
}

private int getActiveClients( int clientState )
{
return clientState & ~STATE_BIT_MASK;
}

private int stateWithNewStatus( int clientState, int newStatus )
{
return newStatus | getActiveClients( clientState );
}

private int statusWithUpdatedClients( int clientState, int delta )
{
return getStatus( clientState ) | (getActiveClients( clientState ) + delta);
}
}
Expand Up @@ -35,8 +35,8 @@
* <p>
* Multiple locks on the same resource held by the same transaction requires the
* transaction to invoke the release lock method multiple times. If a tx has
* invoked {@link #getReadLock(Object, Transaction)} on the same resource x times in
* a row it must invoke {@link #releaseReadLock(Object, Transaction)} x times to release
* invoked {@link #getReadLock(Object, Object)} on the same resource x times in
* a row it must invoke {@link #releaseReadLock(Object, Object)} x times to release
* all the locks. Same for write locks correspondingly.
* <p>
* LockManager just maps locks to resources and they do all the hard work
Expand All @@ -49,12 +49,16 @@ public interface LockManager
* transaction. If read lock can't be acquired the transaction will wait for
* the transaction until it can acquire it. If waiting leads to dead lock a
* {@link DeadlockDetectedException} will be thrown.
* Waiting 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
*
* @param resource the resource to lock
* @throws DeadlockDetectedException if a deadlock is detected, or prevented rather
* @throws IllegalResourceException if an illegal resource is supplied
* @return true is lock was acquired, false otherwise
*/
void getReadLock( Object resource, Object tx )
boolean getReadLock( Object resource, Object tx )
throws DeadlockDetectedException, IllegalResourceException;

/**
Expand All @@ -64,6 +68,7 @@ void getReadLock( Object resource, Object tx )
*
* @param resource the resource
* @throws IllegalResourceException if an illegal resource is supplied
* @return true is lock was acquired, false otherwise
*/
boolean tryReadLock( Object resource, Object tx )
throws IllegalResourceException;
Expand All @@ -73,12 +78,16 @@ boolean tryReadLock( Object resource, Object tx )
* transaction. If write lock can't be acquired the transaction will wait
* for the lock until it can acquire it. If waiting leads to dead lock a
* {@link DeadlockDetectedException} will be thrown.
* Waiting 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
*
* @param resource the resource
* @throws DeadlockDetectedException if a deadlock is detected, or prevented rather
* @throws IllegalResourceException if an illegal resource is supplied
* @return true is lock was acquired, false otherwise
*/
void getWriteLock( Object resource, Object tx )
boolean getWriteLock( Object resource, Object tx )
throws DeadlockDetectedException, IllegalResourceException;

/**
Expand All @@ -88,6 +97,7 @@ void getWriteLock( Object resource, Object tx )
*
* @param resource the resource
* @throws IllegalResourceException if an illegal resource is supplied
* @return true is lock was acquired, false otherwise
*/
boolean tryWriteLock( Object resource, Object tx )
throws IllegalResourceException;
Expand All @@ -114,13 +124,13 @@ void releaseWriteLock( Object resource, Object tx )

/**
* @return number of deadlocks that have been detected and prevented.
* @see #getWriteLock(Object, Transaction) and {@link #getReadLock(Object, Transaction)}.
* @see #getWriteLock(Object, Object) and {@link #getReadLock(Object, Object)}.
*/
long getDetectedDeadlockCount();

/**
* Utility method for debugging. Dumps info to {@code logger} about txs having locks on resources.
*/
public void dumpLocksOnResource( final Object resource, Logger logger );
void dumpLocksOnResource( final Object resource, Logger logger );

}

0 comments on commit cdfce78

Please sign in to comment.