Return-Path: X-Original-To: apmail-river-commits-archive@www.apache.org Delivered-To: apmail-river-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 443057860 for ; Tue, 27 Dec 2011 07:23:59 +0000 (UTC) Received: (qmail 68501 invoked by uid 500); 27 Dec 2011 07:23:59 -0000 Delivered-To: apmail-river-commits-archive@river.apache.org Received: (qmail 68461 invoked by uid 500); 27 Dec 2011 07:23:56 -0000 Mailing-List: contact commits-help@river.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@river.apache.org Delivered-To: mailing list commits@river.apache.org Received: (qmail 68454 invoked by uid 99); 27 Dec 2011 07:23:54 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 Dec 2011 07:23:54 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 Dec 2011 07:23:51 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 7311F23888D2; Tue, 27 Dec 2011 07:23:29 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1224874 - /river/jtsk/skunk/peterConcurrentPolicy/src/com/sun/jini/start/AggregatePolicyProvider.java Date: Tue, 27 Dec 2011 07:23:29 -0000 To: commits@river.apache.org From: peter_firmstone@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20111227072329.7311F23888D2@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 trustGetCCL + = RC.concurrentMap( + new ConcurrentHashMap,Referrer>(), + Ref.WEAK_IDENTITY, Ref.STRONG); + private static final ProtectionDomain myDomain + = AccessController.doPrivileged( + new PrivilegedAction() { + public ProtectionDomain run() { + return AggregatePolicyProvider.class.getProtectionDomain(); + } + }); + + private final ConcurrentMap subPolicies = + RC.concurrentMap( + new ConcurrentHashMap,Referrer>(), + 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 subPolicyChildClassLoaderCache = // put protected by policyRead + RC.concurrentMap( // clear protected by policyWrite + new ConcurrentHashMap,Referrer>(), + 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 AggregatePolicyProvider 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() { + 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() { + 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() { + 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() { + public ClassLoader run() { Thread t = Thread.currentThread(); ClassLoader old = null; if (force || ldr != (old = t.getContextClassLoader())) {