river-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From peter_firmst...@apache.org
Subject svn commit: r1224874 - /river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
Date Tue, 27 Dec 2011 07:23:29 GMT
Author: peter_firmstone
Date: Tue Dec 27 07:23:29 2011
New Revision: 1224874

URL: http://svn.apache.org/viewvc?rev=1224874&view=rev
Log:
River-323 Fix deadlock experienced during debugging.

Modified:
    river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java?rev=1224874&r1=1224873&r2=1224874&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
(original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java
Tue Dec 27 07:23:29 2011
@@ -18,7 +18,6 @@
 
 package com.sun.jini.start;
 
-import com.sun.jini.collection.WeakIdentityMap;
 import java.lang.reflect.Method;
 import java.security.AccessControlContext;
 import java.security.AccessController;
@@ -34,12 +33,18 @@ import java.security.Policy;
 import java.security.ProtectionDomain;
 import java.security.Security;
 import java.security.SecurityPermission;
-import java.util.Map;
-import java.util.WeakHashMap;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import net.jini.security.SecurityContext;
 import net.jini.security.policy.DynamicPolicy;
 import net.jini.security.policy.PolicyInitializationException;
 import net.jini.security.policy.SecurityContextSource;
+import org.apache.river.impl.util.RC;
+import org.apache.river.impl.util.Ref;
+import org.apache.river.impl.util.Referrer;
 
 /**
  * Security policy provider which supports associating security sub-policies
@@ -76,17 +81,33 @@ public class AggregatePolicyProvider 
     private static final String defaultMainPolicyClass =
 	"net.jini.security.policy.DynamicPolicyProvider";
 
-    private static final Map trustGetCCL = new WeakHashMap();
-    private static final ProtectionDomain myDomain = (ProtectionDomain)
-	AccessController.doPrivileged(new PrivilegedAction() {
-	    public Object run() {
-		return AggregatePolicyProvider.class.getProtectionDomain();
-	    }
-	});
-
-    private WeakIdentityMap subPolicies = new WeakIdentityMap();
-    private WeakIdentityMap subPolicyCache = new WeakIdentityMap();
-    private Policy mainPolicy;
+    private static final ConcurrentMap<Class,Boolean> trustGetCCL
+    = RC.concurrentMap(
+            new ConcurrentHashMap<Referrer<Class>,Referrer<Boolean>>(),

+            Ref.WEAK_IDENTITY, Ref.STRONG);
+    private static final ProtectionDomain myDomain 
+        = AccessController.doPrivileged(
+            new PrivilegedAction<ProtectionDomain>() {
+                 public ProtectionDomain run() {
+                     return AggregatePolicyProvider.class.getProtectionDomain();
+                 }
+             });
+
+    private final ConcurrentMap<ClassLoader,Policy> subPolicies = 
+            RC.concurrentMap(
+            new ConcurrentHashMap<Referrer<ClassLoader>,Referrer<Policy>>(),
+            Ref.WEAK, Ref.STRONG);
+    // The cache is used to avoid repeat security checks, the subPolicies map
+    // cannot be used to cache child ClassLoaders because their policy could
+    // change if a policy is updated.
+    private final ConcurrentMap<ClassLoader,Policy> subPolicyChildClassLoaderCache
= // put protected by policyRead
+            RC.concurrentMap(                                                        // clear
protected by policyWrite
+            new ConcurrentHashMap<Referrer<ClassLoader>,Referrer<Policy>>(),
+            Ref.WEAK, Ref.STRONG);
+    private final ReadWriteLock policyUpdate = new ReentrantReadWriteLock(); 
+    private final Lock policyRead = policyUpdate.readLock();
+    private final Lock policyWrite = policyUpdate.writeLock();
+    private Policy mainPolicy; // protected by policyUpdate
 
     /**
      * Creates a new <code>AggregatePolicyProvider</code> instance, containing
@@ -237,12 +258,11 @@ public class AggregatePolicyProvider 
 	if (sm != null) {
 	    sm.checkPermission(new SecurityPermission("setPolicy"));
 	}
-	synchronized (subPolicies) {
-	    subPolicyCache.clear();
+        policyWrite.lock();
+        try {
 	    if (loader != null) {
 		if (subPolicy != null) {
 		    subPolicies.put(loader, subPolicy);
-		    subPolicyCache.put(loader, subPolicy);
 		} else {
 		    subPolicies.remove(loader);
 		}
@@ -252,7 +272,10 @@ public class AggregatePolicyProvider 
 		}
 		mainPolicy = subPolicy;
 	    }
-	}
+            subPolicyChildClassLoaderCache.clear();
+        } finally {
+            policyWrite.unlock();
+        }
     }
 
     /**
@@ -365,9 +388,7 @@ public class AggregatePolicyProvider 
 	// force class resolution by pre-invoking methods called by implies()
 	trustGetContextClassLoader0(Thread.class);
 	getContextClassLoader();
-	synchronized (subPolicies) {
-	    lookupSubPolicy(ldr);
-	}
+        lookupSubPolicy(ldr);
     }
 
     /**
@@ -376,17 +397,19 @@ public class AggregatePolicyProvider 
     private Policy getCurrentSubPolicy() {
 	final Thread t = Thread.currentThread();
 	if (!trustGetContextClassLoader(t)) {
-	    return mainPolicy;
+            policyRead.lock();
+            try {
+                return mainPolicy;
+            }finally{
+                policyRead.unlock();
+            }
 	}
 	ClassLoader ccl = getContextClassLoader();
-	synchronized (subPolicies) {
-	    Policy policy = (Policy) subPolicyCache.get(ccl);
-	    if (policy == null) {
-		policy = lookupSubPolicy(ccl);
-		subPolicyCache.put(ccl, policy);
-	    }
+	    Policy policy = subPolicies.get(ccl);
+            if (policy == null) policy = subPolicyChildClassLoaderCache.get(ccl);
+	    if (policy == null) policy = lookupSubPolicy(ccl);
 	    return policy;
-	}
+	
     }
 
     /**
@@ -394,17 +417,25 @@ public class AggregatePolicyProvider 
      * should only be called when already synchronized on subPolicies.
      */
     private Policy lookupSubPolicy(final ClassLoader ldr) {
-	assert Thread.holdsLock(subPolicies);
-	return (Policy) AccessController.doPrivileged(
-	    new PrivilegedAction() {
-		public Object run() {
-		    for (ClassLoader l = ldr; l != null; l = l.getParent()) {
-			Policy p = (Policy) subPolicies.get(l);
-			if (p != null) {
-			    return p;
-			}
-		    }
-		    return mainPolicy;
+	return AccessController.doPrivileged(
+	    new PrivilegedAction<Policy>() {
+		public Policy run() {
+                    Policy p = null;
+                    policyRead.lock();
+                    try {
+                        for (ClassLoader l = ldr; l != null; l = l.getParent()) {
+                            p = subPolicies.get(l);
+                            if (p != null) break;
+                        }
+                        if (p == null) p = mainPolicy;
+                        Policy exists =
+                        subPolicyChildClassLoaderCache.putIfAbsent(ldr, p);
+                        if ( exists != null && p != exists ) 
+                            throw new IllegalStateException("Policy Mutation occured");
+                        return p;
+                    }finally{
+                        policyRead.unlock();
+                    }
 		}
 	    });
     }
@@ -413,9 +444,9 @@ public class AggregatePolicyProvider 
      * Returns current context class loader.
      */
     static ClassLoader getContextClassLoader() {
-	return (ClassLoader) AccessController.doPrivileged(
-	    new PrivilegedAction() {
-		public Object run() {
+	return AccessController.doPrivileged(
+	    new PrivilegedAction<ClassLoader>() {
+		public ClassLoader run() {
 		    return Thread.currentThread().getContextClassLoader();
 		}
 	    });
@@ -432,22 +463,20 @@ public class AggregatePolicyProvider 
 	}
 	
 	Boolean b;
-	synchronized (trustGetCCL) {
-	    b = (Boolean) trustGetCCL.get(cl);
-	}
+        b = trustGetCCL.get(cl);
 	if (b == null) {
 	    b = trustGetContextClassLoader0(cl);
-	    synchronized (trustGetCCL) {
-		trustGetCCL.put(cl, b);
-	    }
+//            Boolean existed = 
+            trustGetCCL.putIfAbsent(cl, b);
 	}
 	return b.booleanValue();
     }
 
     private static Boolean trustGetContextClassLoader0(final Class cl) {
-	return (Boolean) AccessController.doPrivileged(new PrivilegedAction() {
-	    public Object run() {
+	return AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
+	    public Boolean run() {
 		try {
+                    @SuppressWarnings("unchecked")
 		    Method m = cl.getMethod(
 			"getContextClassLoader", new Class[0]);
 		    return Boolean.valueOf(m.getDeclaringClass() == Thread.class);
@@ -538,9 +567,9 @@ public class AggregatePolicyProvider 
 	}
 
 	private ClassLoader setCCL(final ClassLoader ldr, final boolean force) {
-	    return (ClassLoader) AccessController.doPrivileged(
-		new PrivilegedAction() {
-		    public Object run() {
+	    return AccessController.doPrivileged(
+		new PrivilegedAction<ClassLoader>() {
+		    public ClassLoader run() {
 			Thread t = Thread.currentThread();
 			ClassLoader old = null;
 			if (force || ldr != (old = t.getContextClassLoader())) {



Mime
View raw message