I don't see the need of locking in methods like "public boolean
hasSubentry(...)". What's the benefit of locking?
A normal use case is IMO that some time after calling the has... method
one will either read or write the cache. But there's no garantee, that
when now read/write the cache is still in the same state as when calling
the has... method. So isn't it just a waste of time to lock in this case?
Curious Felix
On 01/11/2011 04:57 PM, [email protected] wrote:
Author: elecharny
Date: Tue Jan 11 15:57:56 2011
New Revision: 1057707
URL: http://svn.apache.org/viewvc?rev=1057707&view=rev
Log:
Added synchronization around the cache
Modified:
directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
Modified:
directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
URL:
http://svn.apache.org/viewvc/directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java?rev=1057707&r1=1057706&r2=1057707&view=diff
==============================================================================
---
directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
(original)
+++
directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
Tue Jan 11 15:57:56 2011
@@ -23,6 +23,7 @@ package org.apache.directory.server.core
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.directory.shared.ldap.exception.LdapException;
import org.apache.directory.shared.ldap.name.DN;
@@ -44,6 +45,49 @@ public class SubentryCache implements It
/** The Subentry DN cache */
private final DnNode<Subentry[]> dnCache;
+ /** A lock to guarantee the Subentry cache consistency */
+ private ReentrantReadWriteLock mutex = new ReentrantReadWriteLock();
+
+ /**
+ * Get a read-lock on the Subentry cache.
+ * No read operation can be done on the AP cache if this
+ * method is not called before.
+ */
+ public void lockRead()
+ {
+ mutex.readLock().lock();
+ }
+
+
+ /**
+ * Get a write-lock on the Subentry cache.
+ * No write operation can be done on the apCache if this
+ * method is not called before.
+ */
+ public void lockWrite()
+ {
+ mutex.writeLock().lock();
+ }
+
+
+ /**
+ * Release the read-write lock on the AP cache.
+ * This method must be called after having read or modified the
+ * AP cache
+ */
+ public void unlock()
+ {
+ if ( mutex.isWriteLockedByCurrentThread() )
+ {
+ mutex.writeLock().unlock();
+ }
+ else
+ {
+ mutex.readLock().unlock();
+ }
+ }
+
+
/**
* Creates a new instance of SubentryCache with a default maximum size.
*/
@@ -63,10 +107,18 @@ public class SubentryCache implements It
*/
public final Subentry getSubentry( String uuid, AdministrativeRoleEnum
role )
{
- Subentry[] subentries = uuidCache.get( uuid );
- Subentry subentry = subentries[role.getValue()];
-
- return subentry;
+ try
+ {
+ lockRead();
+ Subentry[] subentries = uuidCache.get( uuid );
+ Subentry subentry = subentries[role.getValue()];
+
+ return subentry;
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -78,9 +130,17 @@ public class SubentryCache implements It
*/
public final Subentry[] getSubentries( String uuid )
{
- Subentry[] subentries = uuidCache.get( uuid );
-
- return subentries;
+ try
+ {
+ lockRead();
+ Subentry[] subentries = uuidCache.get( uuid );
+
+ return subentries;
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -93,10 +153,18 @@ public class SubentryCache implements It
*/
public final Subentry getSubentry( DN dn, AdministrativeRoleEnum role )
{
- Subentry[] subentries = dnCache.getElement( dn );
- Subentry subentry = subentries[role.getValue()];
-
- return subentry;
+ try
+ {
+ lockRead();
+ Subentry[] subentries = dnCache.getElement( dn );
+ Subentry subentry = subentries[role.getValue()];
+
+ return subentry;
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -108,9 +176,17 @@ public class SubentryCache implements It
*/
public final Subentry[] getSubentries( DN dn )
{
- Subentry[] subentries = dnCache.getElement( dn );
-
- return subentries;
+ try
+ {
+ lockRead();
+ Subentry[] subentries = dnCache.getElement( dn );
+
+ return subentries;
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -123,9 +199,17 @@ public class SubentryCache implements It
*/
public final Subentry[] removeSubentry( String uuid )
{
- Subentry[] oldSubentry = uuidCache.remove( uuid );
-
- return oldSubentry;
+ try
+ {
+ lockWrite();
+ Subentry[] oldSubentry = uuidCache.remove( uuid );
+
+ return oldSubentry;
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -138,22 +222,30 @@ public class SubentryCache implements It
*/
public final Subentry[] removeSubentry( DN dn ) throws LdapException
{
- Subentry[] oldSubentry = dnCache.getElement( dn );
- dnCache.remove( dn );
-
- // Update the UUID cache
- if ( oldSubentry != null )
- {
- for ( Subentry subentry : oldSubentry )
- {
- if ( subentry != null )
+ try
+ {
+ lockWrite();
+ Subentry[] oldSubentry = dnCache.getElement( dn );
+ dnCache.remove( dn );
+
+ // Update the UUID cache
+ if ( oldSubentry != null )
+ {
+ for ( Subentry subentry : oldSubentry )
{
- uuidCache.remove( subentry.getUuid() );
+ if ( subentry != null )
+ {
+ uuidCache.remove( subentry.getUuid() );
+ }
}
}
+
+ return oldSubentry;
+ }
+ finally
+ {
+ unlock();
}
-
- return oldSubentry;
}
@@ -166,28 +258,36 @@ public class SubentryCache implements It
*/
public Subentry[] addSubentry( DN dn, Subentry subentry ) throws
LdapException
{
- Subentry[] subentries = uuidCache.get( subentry.getUuid() );
-
- if ( subentries == null )
- {
- subentries = new Subentry[4];
- }
-
- subentries[subentry.getAdministrativeRole().getValue()] = subentry;
- Subentry[] oldSubentry = uuidCache.put( subentry.getUuid(), subentries
);
-
- Subentry[] dnSubentries = dnCache.getElement( dn );
-
- if ( dnSubentries != null )
+ try
{
- dnSubentries[subentry.getAdministrativeRole().getValue()] =
subentry;
+ lockWrite();
+ Subentry[] subentries = uuidCache.get( subentry.getUuid() );
+
+ if ( subentries == null )
+ {
+ subentries = new Subentry[4];
+ }
+
+ subentries[subentry.getAdministrativeRole().getValue()] = subentry;
+ Subentry[] oldSubentry = uuidCache.put( subentry.getUuid(),
subentries );
+
+ Subentry[] dnSubentries = dnCache.getElement( dn );
+
+ if ( dnSubentries != null )
+ {
+ dnSubentries[subentry.getAdministrativeRole().getValue()] =
subentry;
+ }
+ else
+ {
+ dnCache.add( dn, subentries );
+ }
+
+ return oldSubentry;
}
- else
+ finally
{
- dnCache.add( dn, subentries );
+ unlock();
}
-
- return oldSubentry;
}
@@ -198,7 +298,15 @@ public class SubentryCache implements It
*/
public boolean hasSubentry( String uuid )
{
- return uuidCache.containsKey( uuid );
+ try
+ {
+ lockRead();
+ return uuidCache.containsKey( uuid );
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -209,7 +317,15 @@ public class SubentryCache implements It
*/
public boolean hasSubentry( DN dn )
{
- return dnCache.hasElement( dn );
+ try
+ {
+ lockRead();
+ return dnCache.hasElement( dn );
+ }
+ finally
+ {
+ unlock();
+ }
}
@@ -222,15 +338,23 @@ public class SubentryCache implements It
*/
public boolean hasSubentry( String uuid, AdministrativeRoleEnum role )
{
- Subentry[] subentries = uuidCache.get( uuid );
-
- if ( subentries == null )
+ try
{
- return false;
+ lockRead();
+ Subentry[] subentries = uuidCache.get( uuid );
+
+ if ( subentries == null )
+ {
+ return false;
+ }
+ else
+ {
+ return subentries[ role.getValue() ] != null;
+ }
}
- else
+ finally
{
- return subentries[ role.getValue() ] != null;
+ unlock();
}
}
@@ -243,15 +367,23 @@ public class SubentryCache implements It
*/
public boolean hasSubentry( DN dn, AdministrativeRoleEnum role )
{
- Subentry[] subentries = dnCache.getElement( dn );
-
- if ( subentries == null )
+ try
{
- return false;
+ lockRead();
+ Subentry[] subentries = dnCache.getElement( dn );
+
+ if ( subentries == null )
+ {
+ return false;
+ }
+ else
+ {
+ return subentries[ role.getValue() ] != null;
+ }
}
- else
+ finally
{
- return subentries[ role.getValue() ] != null;
+ unlock();
}
}
@@ -261,6 +393,14 @@ public class SubentryCache implements It
*/
public Iterator<String> iterator()
{
- return uuidCache.keySet().iterator();
+ try
+ {
+ lockRead();
+ return uuidCache.keySet().iterator();
+ }
+ finally
+ {
+ unlock();
+ }
}
}
