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())) {
