You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2014/11/02 01:47:13 UTC
git commit: o Fix for FC-38,
using read and write locks. When we update the protected data,
we release the read lock acquire the write lock,
check that we should still update the data (another might have done that just
after we released the read lock),
Repository: directory-fortress-core
Updated Branches:
refs/heads/master 51d94f189 -> 40f6d2f28
o Fix for FC-38, using read and write locks. When we update the
protected data, we release the read lock acquire the write lock, check
that we should still update the data (another might have done that just
after we released the read lock), then update the data, acquire the read
lock again, release the write lock and finally release the read lock.
It might look like complicated, because it is complex...
Tests are passing.
Project: http://git-wip-us.apache.org/repos/asf/directory-fortress-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/directory-fortress-core/commit/40f6d2f2
Tree: http://git-wip-us.apache.org/repos/asf/directory-fortress-core/tree/40f6d2f2
Diff: http://git-wip-us.apache.org/repos/asf/directory-fortress-core/diff/40f6d2f2
Branch: refs/heads/master
Commit: 40f6d2f28863af71d767138ffa0e5e152b2ab6c6
Parents: 51d94f1
Author: Emmanuel Lécharny <el...@symas.com>
Authored: Sun Nov 2 01:45:17 2014 +0100
Committer: Emmanuel Lécharny <el...@symas.com>
Committed: Sun Nov 2 01:45:17 2014 +0100
----------------------------------------------------------------------
.../fortress/core/rbac/AdminRoleUtil.java | 53 +++++++++++++++----
.../directory/fortress/core/rbac/HierUtil.java | 53 ++++++++++++++++---
.../directory/fortress/core/rbac/PsoUtil.java | 54 +++++++++++++++----
.../directory/fortress/core/rbac/RoleUtil.java | 53 +++++++++++++++----
.../directory/fortress/core/rbac/UsoUtil.java | 55 ++++++++++++++++----
5 files changed, 219 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/directory-fortress-core/blob/40f6d2f2/src/main/java/org/apache/directory/fortress/core/rbac/AdminRoleUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/directory/fortress/core/rbac/AdminRoleUtil.java b/src/main/java/org/apache/directory/fortress/core/rbac/AdminRoleUtil.java
index cdcafbc..986a731 100755
--- a/src/main/java/org/apache/directory/fortress/core/rbac/AdminRoleUtil.java
+++ b/src/main/java/org/apache/directory/fortress/core/rbac/AdminRoleUtil.java
@@ -23,11 +23,11 @@ package org.apache.directory.fortress.core.rbac;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
+import java.util.concurrent.locks.ReadWriteLock;
import org.jgrapht.graph.SimpleDirectedGraph;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
import org.apache.directory.fortress.core.GlobalIds;
import org.apache.directory.fortress.core.SecurityException;
import org.apache.directory.fortress.core.ValidationException;
@@ -233,8 +233,9 @@ public final class AdminRoleUtil
{
Hier inHier = new Hier( Hier.Type.ROLE );
inHier.setContextId( contextId );
- LOG.info( "loadGraph initializing ADMIN ROLE context [" + inHier.getContextId() + "]" );
+ LOG.info( "loadGraph initializing ADMIN ROLE context [{}]", inHier.getContextId() );
List<Graphable> descendants = null;
+
try
{
descendants = adminRoleP.getAllDescendants( inHier.getContextId() );
@@ -243,13 +244,13 @@ public final class AdminRoleUtil
{
LOG.info( "loadGraph caught SecurityException={}", se );
}
+
Hier hier = HierUtil.loadHier( contextId, descendants );
SimpleDirectedGraph<String, Relationship> graph;
- synchronized ( HierUtil.getLock( contextId, HierUtil.Type.ARLE ) )
- {
- graph = HierUtil.buildGraph( hier );
- }
+
+ graph = HierUtil.buildGraph( hier );
adminRoleCache.put( getKey( contextId ), graph );
+
return graph;
}
@@ -263,13 +264,43 @@ public final class AdminRoleUtil
*/
private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId )
{
- SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) adminRoleCache
- .get( getKey( contextId ) );
- if ( graph == null )
+ ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.ARLE );
+ String key = getKey( contextId );
+
+ try
{
- graph = loadGraph( contextId );
+ hierLock.readLock().lock();
+ SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) adminRoleCache
+ .get( key );
+
+ if ( graph == null )
+ {
+ try
+ {
+ hierLock.readLock().unlock();
+ hierLock.writeLock().lock();
+
+ graph = ( SimpleDirectedGraph<String, Relationship> ) adminRoleCache.get( key );
+
+ if ( graph == null )
+ {
+ graph = loadGraph( contextId );
+ }
+
+ hierLock.readLock().lock();
+ }
+ finally
+ {
+ hierLock.writeLock().unlock();
+ }
+ }
+
+ return graph;
+ }
+ finally
+ {
+ hierLock.readLock().unlock();
}
- return graph;
}
http://git-wip-us.apache.org/repos/asf/directory-fortress-core/blob/40f6d2f2/src/main/java/org/apache/directory/fortress/core/rbac/HierUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/directory/fortress/core/rbac/HierUtil.java b/src/main/java/org/apache/directory/fortress/core/rbac/HierUtil.java
index 1305190..d9ab2ad 100755
--- a/src/main/java/org/apache/directory/fortress/core/rbac/HierUtil.java
+++ b/src/main/java/org/apache/directory/fortress/core/rbac/HierUtil.java
@@ -26,11 +26,12 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.jgrapht.graph.SimpleDirectedGraph;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
import org.apache.directory.fortress.core.GlobalErrIds;
import org.apache.directory.fortress.core.SecurityException;
import org.apache.directory.fortress.core.ValidationException;
@@ -70,6 +71,10 @@ final class HierUtil
private static final Logger LOG = LoggerFactory.getLogger( CLS_NM );
private static final String VERTEX = "Vertex";
+ /** A lock used internally to protect the access to the locks map */
+ private static final ReadWriteLock getLockLock = new ReentrantReadWriteLock();
+
+
/**
* The 'Type' attribute corresponds to what type of hierarchy is being referred to.
*/
@@ -81,7 +86,7 @@ final class HierUtil
PSO
}
- private static final Map<String, Object> synchMap = new HashMap<>();
+ private static final Map<String, ReadWriteLock> synchMap = new HashMap<String, ReadWriteLock>();
/**
@@ -90,15 +95,47 @@ final class HierUtil
* @param type
* @return
*/
- static Object getLock( String contextId, Type type )
+ static ReadWriteLock getLock( String contextId, Type type )
{
- Object synchObj = synchMap.get( getSynchKey( contextId, type ) );
- if ( synchObj == null )
+ String syncKey = getSynchKey( contextId, type );
+
+ try
+ {
+ getLockLock.readLock().lock();
+ ReadWriteLock synchObj = synchMap.get( syncKey );
+
+ if ( synchObj == null )
+ {
+ // Not found, we will create a new one and store it into the map
+ try
+ {
+ getLockLock.readLock().unlock();
+ getLockLock.writeLock().lock();
+
+ // Retry immediately to get the lock from the map, it might have been updated by
+ // another thread while this thread was blocked on the write lock
+ synchObj = synchMap.get( syncKey );
+
+ if ( synchObj == null )
+ {
+ synchObj = new ReentrantReadWriteLock();
+ synchMap.put( syncKey, synchObj );
+ }
+
+ getLockLock.readLock().lock();
+ }
+ finally
+ {
+ getLockLock.writeLock().unlock();
+ }
+ }
+
+ return synchObj;
+ }
+ finally
{
- synchObj = new Object();
- synchMap.put( getSynchKey( contextId, type ), synchObj );
+ getLockLock.readLock().unlock();
}
- return synchObj;
}
http://git-wip-us.apache.org/repos/asf/directory-fortress-core/blob/40f6d2f2/src/main/java/org/apache/directory/fortress/core/rbac/PsoUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/directory/fortress/core/rbac/PsoUtil.java b/src/main/java/org/apache/directory/fortress/core/rbac/PsoUtil.java
index 2e2b937..004cde4 100755
--- a/src/main/java/org/apache/directory/fortress/core/rbac/PsoUtil.java
+++ b/src/main/java/org/apache/directory/fortress/core/rbac/PsoUtil.java
@@ -23,11 +23,14 @@ package org.apache.directory.fortress.core.rbac;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.jgrapht.graph.SimpleDirectedGraph;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
import org.apache.directory.fortress.core.GlobalIds;
import org.apache.directory.fortress.core.SecurityException;
import org.apache.directory.fortress.core.ValidationException;
@@ -219,6 +222,7 @@ public final class PsoUtil
inHier.setContextId( contextId );
LOG.info( "loadGraph initializing PSO context [" + inHier.getContextId() + "]" );
List<Graphable> descendants = null;
+
try
{
OrgUnit orgUnit = new OrgUnit();
@@ -230,13 +234,13 @@ public final class PsoUtil
{
LOG.info( "loadGraph caught SecurityException=" + se );
}
+
Hier hier = HierUtil.loadHier( contextId, descendants );
SimpleDirectedGraph<String, Relationship> graph;
- synchronized ( HierUtil.getLock( contextId, HierUtil.Type.PSO ) )
- {
- graph = HierUtil.buildGraph( hier );
- }
+
+ graph = HierUtil.buildGraph( hier );
psoCache.put( getKey( contextId ), graph );
+
return graph;
}
@@ -247,13 +251,43 @@ public final class PsoUtil
*/
private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId )
{
- SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) psoCache
- .get( getKey( contextId ) );
- if ( graph == null )
+ ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.PSO );
+ String key = getKey( contextId );
+
+ try
+ {
+ hierLock.readLock().lock();
+ SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) psoCache
+ .get( key );
+
+ if ( graph == null )
+ {
+ try
+ {
+ hierLock.readLock().unlock();
+ hierLock.writeLock().lock();
+
+ graph = ( SimpleDirectedGraph<String, Relationship> ) psoCache.get( key );
+
+ if ( graph == null )
+ {
+ graph = loadGraph( contextId );
+ }
+
+ hierLock.readLock().lock();
+ }
+ finally
+ {
+ hierLock.writeLock().unlock();
+ }
+ }
+
+ return graph;
+ }
+ finally
{
- graph = loadGraph( contextId );
+ hierLock.readLock().unlock();
}
- return graph;
}
http://git-wip-us.apache.org/repos/asf/directory-fortress-core/blob/40f6d2f2/src/main/java/org/apache/directory/fortress/core/rbac/RoleUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/directory/fortress/core/rbac/RoleUtil.java b/src/main/java/org/apache/directory/fortress/core/rbac/RoleUtil.java
index 67ce3c5..5de6533 100755
--- a/src/main/java/org/apache/directory/fortress/core/rbac/RoleUtil.java
+++ b/src/main/java/org/apache/directory/fortress/core/rbac/RoleUtil.java
@@ -23,11 +23,11 @@ package org.apache.directory.fortress.core.rbac;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
+import java.util.concurrent.locks.ReadWriteLock;
import org.jgrapht.graph.SimpleDirectedGraph;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
import org.apache.directory.fortress.core.GlobalIds;
import org.apache.directory.fortress.core.SecurityException;
import org.apache.directory.fortress.core.ValidationException;
@@ -305,6 +305,7 @@ public final class RoleUtil
inHier.setContextId( contextId );
LOG.info( "loadGraph initializing ROLE context [" + inHier.getContextId() + "]" );
List<Graphable> descendants = null;
+
try
{
descendants = roleP.getAllDescendants( inHier.getContextId() );
@@ -313,13 +314,13 @@ public final class RoleUtil
{
LOG.info( "loadGraph caught SecurityException=" + se );
}
+
Hier hier = HierUtil.loadHier( contextId, descendants );
SimpleDirectedGraph<String, Relationship> graph;
- synchronized ( HierUtil.getLock( contextId, HierUtil.Type.ROLE ) )
- {
- graph = HierUtil.buildGraph( hier );
- }
+
+ graph = HierUtil.buildGraph( hier );
roleCache.put( getKey( contextId ), graph );
+
return graph;
}
@@ -332,10 +333,12 @@ public final class RoleUtil
private static String getKey( String contextId )
{
String key = HierUtil.Type.ROLE.toString();
+
if ( VUtil.isNotNullOrEmpty( contextId ) && !contextId.equalsIgnoreCase( GlobalIds.NULL ) )
{
key += ":" + contextId;
}
+
return key;
}
@@ -347,12 +350,42 @@ public final class RoleUtil
*/
private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId )
{
- SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache
- .get( getKey( contextId ) );
- if ( graph == null )
+ ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.ROLE );
+ String key = getKey( contextId ) ;
+
+ try
{
- graph = loadGraph( contextId );
+ hierLock.readLock().lock();
+ SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache
+ .get( key );
+
+ if ( graph == null )
+ {
+ try
+ {
+ hierLock.readLock().unlock();
+ hierLock.writeLock().lock();
+
+ graph = ( SimpleDirectedGraph<String, Relationship> ) roleCache.get( key );
+
+ if ( graph == null )
+ {
+ graph = loadGraph( contextId );
+ }
+
+ hierLock.readLock().lock();
+ }
+ finally
+ {
+ hierLock.writeLock().unlock();
+ }
+ }
+
+ return graph;
+ }
+ finally
+ {
+ hierLock.readLock().unlock();
}
- return graph;
}
}
http://git-wip-us.apache.org/repos/asf/directory-fortress-core/blob/40f6d2f2/src/main/java/org/apache/directory/fortress/core/rbac/UsoUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/directory/fortress/core/rbac/UsoUtil.java b/src/main/java/org/apache/directory/fortress/core/rbac/UsoUtil.java
index 6a7c30b..fe04906 100755
--- a/src/main/java/org/apache/directory/fortress/core/rbac/UsoUtil.java
+++ b/src/main/java/org/apache/directory/fortress/core/rbac/UsoUtil.java
@@ -23,11 +23,11 @@ package org.apache.directory.fortress.core.rbac;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
+import java.util.concurrent.locks.ReadWriteLock;
import org.jgrapht.graph.SimpleDirectedGraph;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
import org.apache.directory.fortress.core.GlobalIds;
import org.apache.directory.fortress.core.SecurityException;
import org.apache.directory.fortress.core.ValidationException;
@@ -145,6 +145,7 @@ public final class UsoUtil
{
// create Set with case insensitive comparator:
Set<String> iOUs = new TreeSet<>( String.CASE_INSENSITIVE_ORDER );
+
if ( VUtil.isNotNullOrEmpty( ous ) )
{
for ( OrgUnit ou : ous )
@@ -152,10 +153,14 @@ public final class UsoUtil
String name = ou.getName();
iOUs.add( name );
Set<String> parents = HierUtil.getAscendants( name, getGraph( contextId ) );
+
if ( VUtil.isNotNullOrEmpty( parents ) )
+ {
iOUs.addAll( parents );
+ }
}
}
+
return iOUs;
}
@@ -211,6 +216,7 @@ public final class UsoUtil
Hier inHier = new Hier( Hier.Type.ROLE );
inHier.setContextId( contextId );
LOG.info( "loadGraph initializing USO context [" + inHier.getContextId() + "]" );
+
List<Graphable> descendants = null;
try
{
@@ -223,13 +229,13 @@ public final class UsoUtil
{
LOG.info( "loadGraph caught SecurityException=" + se );
}
+
Hier hier = HierUtil.loadHier( contextId, descendants );
SimpleDirectedGraph<String, Relationship> graph;
- synchronized ( HierUtil.getLock( contextId, HierUtil.Type.USO ) )
- {
- graph = HierUtil.buildGraph( hier );
- }
+
+ graph = HierUtil.buildGraph( hier );
usoCache.put( getKey( contextId ), graph );
+
return graph;
}
@@ -240,13 +246,42 @@ public final class UsoUtil
*/
private static SimpleDirectedGraph<String, Relationship> getGraph( String contextId )
{
- SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) usoCache
- .get( getKey( contextId ) );
- if ( graph == null )
+ ReadWriteLock hierLock = HierUtil.getLock( contextId, HierUtil.Type.USO );
+ String key = getKey( contextId );
+
+ try
{
- graph = loadGraph( contextId );
+ hierLock.readLock().lock();
+ SimpleDirectedGraph<String, Relationship> graph = ( SimpleDirectedGraph<String, Relationship> ) usoCache
+ .get( key );
+
+ if ( graph == null )
+ {
+ try
+ {
+ hierLock.readLock().unlock();
+ hierLock.writeLock().lock();
+
+ graph = ( SimpleDirectedGraph<String, Relationship> ) usoCache.get( key );
+
+ if ( graph == null )
+ {
+ graph = loadGraph( contextId );
+ }
+ hierLock.readLock().lock();
+ }
+ finally
+ {
+ hierLock.writeLock().unlock();
+ }
+ }
+
+ return graph;
+ }
+ finally
+ {
+ hierLock.readLock().unlock();
}
- return graph;
}