You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by pe...@apache.org on 2012/01/09 14:12:54 UTC

svn commit: r1229137 [2/3] - in /river/jtsk/skunk/peterConcurrentPolicy: qa/harness/policy/ qa/src/com/sun/jini/qa/harness/ qa/src/com/sun/jini/qa/resources/ qa/src/com/sun/jini/test/impl/start/ qa/src/com/sun/jini/test/impl/start/aggregatepolicyprovid...

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/ConcurrentPolicyFile.java Mon Jan  9 13:12:52 2012
@@ -34,32 +34,27 @@ import java.security.CodeSource;
 import java.security.Guard;
 import java.security.Permission;
 import java.security.PermissionCollection;
+import java.security.Permissions;
 import java.security.Policy;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
 import java.security.ProtectionDomain;
 import java.security.SecurityPermission;
+import java.security.UnresolvedPermission;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.Enumeration;
-import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
+import java.util.NavigableSet;
 import java.util.Properties;
-import java.util.Set;
-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.ConcurrentPermissions;
+import java.util.TreeSet;
+import net.jini.security.PermissionComparator;
 import org.apache.river.api.security.PermissionGrant;
 import org.apache.river.impl.security.policy.util.DefaultPolicyParser;
 import org.apache.river.impl.security.policy.util.PolicyParser;
 import org.apache.river.impl.security.policy.util.PolicyUtils;
-import org.apache.river.impl.util.Ref;
-import org.apache.river.impl.util.RC;
-import org.apache.river.impl.util.Referrer;
 
 
 /**
@@ -169,7 +164,7 @@ import org.apache.river.impl.util.Referr
  *      String)
  */
 
-public class ConcurrentPolicyFile extends Policy {
+public class ConcurrentPolicyFile extends Policy implements ConcurrentPolicy {
 
     /**
      * System property for dynamically added policy location.
@@ -181,72 +176,63 @@ public class ConcurrentPolicyFile extend
      */
     private static final String POLICY_URL_PREFIX = "policy.url."; //$NON-NLS-1$
     
-    private final Lock rl;
+    // Reference must be defensively copied before access, once published, never mutated.
+    private volatile PermissionGrant [] grantArray;
     
-    private final Lock wl;
-    
-    private final Collection<PermissionGrant> grants ; // protected by rwl
-
-    // Calculated Permissions cache, organized as
-    // Map{Object->Collection&lt;Permission&gt;}.
-    // The Object is a ProtectionDomain, a CodeSource or
-    // any other permissions-granted entity.
-    private final ConcurrentMap<Object, Collection<Permission>> cache;
-    
-    private final ConcurrentMap<ProtectionDomain,PermissionCollection> impliesCache;
-
     // A specific parser for a particular policy file format.
     private final PolicyParser parser;
     
-    private static final Guard guard = 
-            new SecurityPermission("getPolicy");
+    private static final Guard guard = new SecurityPermission("getPolicy");
+    
+    private final ProtectionDomain myDomain;
+    
+    private final Comparator<Permission> comparator;
+    
+    // reference must be defensively copied before access, once published, never mutated.
+    private volatile PermissionCollection myPermissions;
     
     /**
      * Default constructor, equivalent to
      * <code>ConcurrentPolicyFile(new DefaultPolicyParser())</code>.
      */
     public ConcurrentPolicyFile() throws PolicyInitializationException {
-        this(new DefaultPolicyParser());
+        this(new DefaultPolicyParser(), new PermissionComparator());
     }
 
     /**
      * Extension constructor for plugging-in a custom parser.
-     * @param dpr 
+     * @param dpr
+     * @param comp Comparator to compare permissions. 
      */
-    protected ConcurrentPolicyFile(PolicyParser dpr) throws PolicyInitializationException {
+    protected ConcurrentPolicyFile(PolicyParser dpr, Comparator<Permission> comp) throws PolicyInitializationException {
         guard.checkGuard(null);
         parser = dpr;
-        ReadWriteLock rwl = new ReentrantReadWriteLock();
-        rl = rwl.readLock();
-        wl = rwl.writeLock();
-        grants = new ArrayList<PermissionGrant>(120);
-        ConcurrentMap<Referrer<Object>, Referrer<Collection<Permission>>> cacheInternal 
-                = new ConcurrentHashMap<Referrer<Object>, Referrer<Collection<Permission>>>(120);
-        cache = RC.concurrentMap(cacheInternal, Ref.WEAK_IDENTITY, Ref.SOFT);
-        ConcurrentMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>> impInternal
-                = new ConcurrentHashMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>>(80);
-        impliesCache = RC.concurrentMap(impInternal, Ref.WEAK_IDENTITY, Ref.SOFT);
+        myDomain = this.getClass().getProtectionDomain();
+        comparator = comp;
         /*
          * The bootstrap policy makes implies decisions until this constructor
          * has returned.  We don't need to lock.
          */
         try {
-            initialize();
-            ensureDependenciesResolved();
+            // Bug 4911907, do we need to do anything more?
+            // The permissions for this domain must be retrieved before
+            // construction is complete and this policy takes over.
+            initialize(); // Instantiates myPermissions.
         } catch (SecurityException e){
             throw e;
         } catch (Exception e){
             throw new PolicyInitializationException("PolicyInitialization failed", e);
         }
     }
-
-    private void ensureDependenciesResolved() {
-        // Investigate bug 4911907, do we need to do anything?
-        // From the work around above, we might not need to do anything.
-        // But these actions prevent the JVM from delaying classloading
-        // of required classes.
-        ProtectionDomain own = this.getClass().getProtectionDomain();
-        implies(own, new AllPermission());
+    
+    private PermissionCollection convert(NavigableSet<Permission> permissions){
+        PermissionCollection pc = new Permissions();
+        // The descending iterator is for SocketPermission.
+        Iterator<Permission> it = permissions.descendingIterator();
+        while (it.hasNext()) {
+            pc.add(it.next());
+        }
+        return pc;
     }
 
     /**
@@ -262,112 +248,134 @@ public class ConcurrentPolicyFile extend
      */
     @Override
     public PermissionCollection getPermissions(ProtectionDomain pd) {
-        /* Permissions is used in preference to ConcurrentPermissions
-         * due the correctness of elements(), ConcurrentPermission's
-         * cannot guarantee that the Enumeration returned by elements()
-         * will be current and copy them, but the Enumeration returned
-         * by Permissions will instead throw a ConcurrentModificationException.
-         */
-        //if (pd == null) return new Permissions();
-        /* When write lock holds thread it will already have this lock
-         * allowing priviledged domain security checks to be performed.
-         */
-        rl.lock();
-        try {
-//            PermissionCollection perms = 
-//                    pd != null ? impliesCache.get(pd): null;
-//            if (perms != null) return perms;
-            PermissionCollection perms = new ConcurrentPermissions();
-                Iterator<PermissionGrant> it = grants.iterator();
-                while (it.hasNext()){
-                    PermissionGrant ge = it.next();
-                    if (ge.implies(pd)){
-                        Collection<Permission> c = ge.getPermissions();
-                        Iterator<Permission> i = c.iterator();
-                        while (i.hasNext()){
-                            Permission p = i.next();
-                            perms.add(p);
-                            // Don't stuff around finish early if you can.
-                            if (p instanceof AllPermission) return perms;
-                        }
-                    }
+        NavigableSet<Permission> perms = new TreeSet<Permission>(comparator);
+        PermissionGrant [] grantRefCopy = grantArray;
+        int l = grantRefCopy.length;
+        for ( int j =0; j < l; j++ ){
+            PermissionGrant ge = grantRefCopy[j];
+            if (ge.implies(pd)){
+                if (ge.isPrivileged()){// Don't stuff around finish early if you can.
+                    PermissionCollection pc = new Permissions();
+                    pc.add(new AllPermission());
+                    return pc;
+                }
+                Collection<Permission> c = ge.getPermissions();
+                Iterator<Permission> i = c.iterator();
+                while (i.hasNext()){
+                    Permission p = i.next();
+                    perms.add(p);
                 }
-            // Don't forget to merge the static Permissions.
-            PermissionCollection staticPC = null;
-            if (pd != null) {
-                staticPC = pd.getPermissions();
-                if (staticPC != null){
-                    Enumeration<Permission> e = staticPC.elements();
-                    while (e.hasMoreElements()){
-                        perms.add(e.nextElement());
+            }
+        }
+        // Don't forget to merge the static Permissions.
+        PermissionCollection staticPC = null;
+        if (pd != null) {
+            staticPC = pd.getPermissions();
+            if (staticPC != null){
+                Enumeration<Permission> e = staticPC.elements();
+                while (e.hasMoreElements()){
+                    Permission p = e.nextElement();
+                    if (p instanceof AllPermission) {
+                        PermissionCollection pc = new Permissions();
+                        pc.add(p);
+                        return pc;
                     }
+                    perms.add(p);
                 }
             }
-//            if (pd != null) impliesCache.put(pd, perms); //Overwrite older.
-            return perms;
-        }finally{
-            rl.unlock();
         }
-       
+        return convert(perms);
     }
 
     /**
      * Returns collection of permissions allowed for the codesource 
      * according to the policy. 
      * The evaluation assumes that current principals are undefined.
+     * 
+     * This returns a java.security.Permissions collection, which allows
+     * ProtectionDomain to optimise for the AllPermission case, which avoids
+     * unnecessarily consulting the policy.
+     * 
+     * If constructed with the four argument constructor, ProtectionDomain.implies
+     * first consults the Policy, then it's own internal Permissions collection,
+     * unless it has AllPermission, in which case it returns true without
+     * consulting the policy.
+     * 
      * @param cs CodeSource
      * @see CodeSource
      */
     @Override
     public PermissionCollection getPermissions(CodeSource cs) {
         if (cs == null) throw new NullPointerException("CodeSource cannot be null");
-        rl.lock();
-        try {
-            Collection<Permission> pc = cache.get(cs); // saves new object creation.
-            if (pc != null){
-                return PolicyUtils.toPermissionCollection(pc);
-            }
-            // Just because the new object is contained within a ConcurrentMap
-            // doesn't mean it doesn't need to be synchronized!
-            // However it isn't modified again after being added to the cache.
-            pc = new HashSet<Permission>();
-            Iterator<PermissionGrant> it = grants.iterator();
-            while (it.hasNext()) {
-                PermissionGrant ge = it.next();
-    //                if (ge.impliesPrincipals(null)
-    //                    && ge.impliesCodeSource(cs)) {
-                if (ge.implies(cs,null )){
-                    Collection<Permission> permCol = ge.getPermissions();
-                    Permission[] perm = permCol.toArray(new Permission [permCol.size()]);
-                    pc.addAll(Arrays.asList(perm));
+        NavigableSet<Permission> perms = new TreeSet<Permission>(comparator);
+//        PermissionCollection perms = new ConcurrentPermissions();
+        // for ProtectionDomain AllPermission optimisation.
+        PermissionGrant [] grantRefCopy = grantArray;
+        int l = grantRefCopy.length;
+        for ( int j =0; j < l; j++ ){
+            PermissionGrant ge = grantRefCopy[j];
+            if (ge.implies(cs, null)){ // No Principal's
+                if (ge.isPrivileged()){// Don't stuff around finish early if you can.
+                    PermissionCollection pc = new Permissions();
+                    pc.add(new AllPermission());
+                    return pc;
+                }
+                Collection<Permission> c = ge.getPermissions();
+                Iterator<Permission> i = c.iterator();
+                while (i.hasNext()){
+                    Permission p = i.next();
+                    perms.add(p);
                 }
             }
-            cache.put(cs, pc); // replace existing.
-            return PolicyUtils.toPermissionCollection(pc);
-        } finally {
-            rl.unlock();
         }
+        return convert(perms);
     }
     
     @Override
     public boolean implies(ProtectionDomain domain, Permission permission) {
         if (permission == null) throw new NullPointerException("permission not allowed to be null");
-        rl.lock();
-        try {
-            PermissionCollection pc = 
-                        domain != null ? impliesCache.get(domain): null;
-            if (pc == null){
-                pc = getPermissions(domain);
-                if (domain != null){
-                    PermissionCollection existed = impliesCache.putIfAbsent(domain, pc);
-                    // Don't bother replacing pc with existed, we're not mutating
-                    // so it doesn't matter.
+        if (domain == myDomain) {
+            PermissionCollection pc = myPermissions;
+            return pc.implies(permission);
+        }
+        Class klass = permission.getClass();
+        // Need to have a list of Permission's we can sort if permission is SocketPermission.
+        NavigableSet<Permission> perms = new TreeSet<Permission>(comparator);
+        PermissionGrant [] grantRefCopy = grantArray;
+        int l = grantRefCopy.length;
+        for ( int j =0; j < l; j++ ){
+            PermissionGrant ge = grantRefCopy[j];
+            if (ge.implies(domain)){
+                if (ge.isPrivileged()) return true; // Don't stuff around finish early if you can.
+                Collection<Permission> c = ge.getPermissions();
+                Iterator<Permission> i = c.iterator();
+                while (i.hasNext()){
+                    Permission p = i.next();
+                    // Don't make it larger than necessary.
+                    if (klass.isInstance(permission) || permission instanceof UnresolvedPermission){
+                        perms.add(p);
+                    }
                 }
             }
-            return pc.implies(permission);
-        }finally{
-            rl.unlock();
         }
+        // Don't forget to merge the static Permissions.
+        PermissionCollection staticPC = null;
+        if (domain != null) {
+            staticPC =domain.getPermissions();
+            if (staticPC != null){
+                Enumeration<Permission> e = staticPC.elements();
+                while (e.hasMoreElements()){
+                    Permission p = e.nextElement();
+                    // return early if possible.
+                    if (p instanceof AllPermission ) return true;
+                    // Don't make it larger than necessary, but don't worry about duplicates either.
+                    if (klass.isInstance(permission) || permission instanceof UnresolvedPermission){
+                        perms.add(p);
+                    }
+                }
+            }
+        }
+        return convert(perms).implies(permission);
     }
 
     /**
@@ -379,22 +387,19 @@ public class ConcurrentPolicyFile extend
      */
     @Override
     public void refresh() {
-        if ( !wl.tryLock() ) return; // If another thread has the lock, policy is undergoing an update.
         try {
             initialize();
         } catch (Exception ex) {
-            ex.printStackTrace(System.err);
-        } finally {
-            wl.unlock();
+            System.err.println(ex);
         }
     }
     
     private void initialize() throws Exception{
         try {
-            Set<PermissionGrant> fresh = AccessController.doPrivileged( 
-                new PrivilegedExceptionAction<Set<PermissionGrant>>(){
-                    public Set<PermissionGrant> run() throws SecurityException {
-                        Set<PermissionGrant> fresh = new HashSet<PermissionGrant>(120);
+            Collection<PermissionGrant> fresh = AccessController.doPrivileged( 
+                new PrivilegedExceptionAction<Collection<PermissionGrant>>(){
+                    public Collection<PermissionGrant> run() throws SecurityException {
+                        Collection<PermissionGrant> fresh = new ArrayList<PermissionGrant>(120);
                         Properties system = System.getProperties();
                         system.setProperty("/", File.separator); //$NON-NLS-1$
                         URL[] policyLocations = PolicyUtils.getPolicyURLs(system,
@@ -407,7 +412,6 @@ public class ConcurrentPolicyFile extend
                             try {
                                 Collection<PermissionGrant> pc = null;
                                 pc = parser.parse(policyLocations[i], system);
-                                    grants.addAll(pc); 
                                 fresh.addAll(pc);
                             } catch (Exception e){
                                 // It's best to let a SecurityException bubble up
@@ -419,23 +423,40 @@ public class ConcurrentPolicyFile extend
                                 }
                                 // ignore.
                             }
-                        } 
+                        }
                         return fresh;
                     }
                 }
             );
-            // Even though grant's are removed, it's not that important
-            // as policies generally are additive,
-            //static ProtectionDomains call Policy.getPermissions(CodeSource)
-            // references escape etc.
-            grants.retainAll(fresh); // removes all that are no longer valid.
-            cache.clear(); // Clear the cache.
-            impliesCache.clear();
+            // Volatile reference, publish after mutation complete.
+            grantArray = fresh.toArray(new PermissionGrant[fresh.size()]);
+            myPermissions = getPermissions(myDomain);
         }catch (PrivilegedActionException e){
             Throwable t = e.getCause();
             if ( t instanceof Exception ) throw (Exception) t;
             throw e;
         }
     }
+
+    public boolean isConcurrent() {
+        return true;
+    }
+    
+    public PermissionGrant[] getPermissionGrants(ProtectionDomain pd) {
+        PermissionGrant [] grants = grantArray; // copy volatile reference target.
+        int l = grants.length;
+        List<PermissionGrant> applicable = new ArrayList<PermissionGrant>(l); // Always too large, never too small.
+        for (int i =0; i < l; i++){
+            if (grants[i].implies(pd)){
+                applicable.add(grants[i]);
+            }
+        }
+        return applicable.toArray(new PermissionGrant[applicable.size()]);
+    }
     
+    public PermissionGrant[] getPermissionGrants() {
+        PermissionGrant [] grants = grantArray; // copy volatile reference target.
+        return grants.clone();
+    }
+
 }

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/DynamicPolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/DynamicPolicyProvider.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/DynamicPolicyProvider.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/net/jini/security/policy/DynamicPolicyProvider.java Mon Jan  9 13:12:52 2012
@@ -36,14 +36,17 @@ import java.security.PrivilegedAction;
 import java.security.PrivilegedExceptionAction;
 import java.security.ProtectionDomain;
 import java.security.Security;
+import java.security.UnresolvedPermission;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.NavigableSet;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
@@ -168,7 +171,8 @@ import org.apache.river.impl.util.Referr
 
 public class DynamicPolicyProvider extends Policy implements RemotePolicy, 
         RevokeableDynamicPolicy {
-     private static final String basePolicyClassProperty =
+    private static final Permission ALL_PERMISSION = new AllPermission();
+    private static final String basePolicyClassProperty =
 	"net.jini.security.policy.DynamicPolicyProvider.basePolicyClass";
     private static final String defaultBasePolicyClass =
             "net.jini.security.policy.ConcurrentPolicyFile";
@@ -180,6 +184,7 @@ public class DynamicPolicyProvider exten
 	});
     private static final String revocationSupported = 
             "net.jini.security.policy.DynamicPolicyProvider.revocation";
+    private static final Logger logger = Logger.getLogger("net.jini.security.policy");
     
     /* 
      * Copy referent before use.
@@ -197,18 +202,20 @@ public class DynamicPolicyProvider exten
     private final Object grantLock;
     private final Policy basePolicy; // refresh protected by transactionWriteLock
     /* cache of ProtectionDomain and their Permissions */
-    private final ConcurrentMap<ProtectionDomain, PermissionCollection> cache; // protected by transactionWriteLock
+//    private final ConcurrentMap<ProtectionDomain, PermissionCollection> cache; // protected by transactionWriteLock
     /* A transaction ID to avoid updating the cache with old information
      * after it has been cleared */
-    private final Lock transactionWriteLock; // Lock to protect cache clear and transactionID write.
-    private final Lock transactionReadLock; // Lock to protect cache put and transactionID reads.
-    private int transactionID; // Protected by transaction locks 
+//    private final Lock transactionWriteLock; // Lock to protect cache clear and transactionID write.
+//    private final Lock transactionReadLock; // Lock to protect cache put and transactionID reads.
+//    private int transactionID; // Protected by transaction locks 
     // DynamicPolicy grant's for Proxy's.
     private final Collection<PermissionGrant> dynamicPolicyGrants;
     private final boolean basePolicyIsDynamic; // Don't use cache if true.
     private final boolean revokeable;
     private final boolean basePolicyIsRemote;
-    private static final Logger logger = Logger.getLogger("net.jini.security.policy");
+    private final boolean basePolicyIsConcurrent;
+    private final Comparator<Permission> comparator = new PermissionComparator();
+    
     private final boolean loggable;
     // do something about some domain permissions for this domain so we can 
     // avoid dead locks due to bug 4911907
@@ -217,6 +224,9 @@ public class DynamicPolicyProvider exten
     private final Permission implementsPermissionGrant;
     private final Guard protectionDomainPermission;
     
+    private final ProtectionDomain policyDomain;
+    private final PermissionCollection policyPermissions;
+    
     /**
      * Creates a new <code>DynamicPolicyProvider</code> instance that wraps a
      * default underlying policy.  The underlying policy is created as follows:
@@ -281,9 +291,9 @@ public class DynamicPolicyProvider exten
          * referenced, in the case where the client hangs onto objects
          * recieved from it.
          */
-        ConcurrentMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>> internal 
-                = new ConcurrentHashMap<Referrer<ProtectionDomain>,Referrer<PermissionCollection>>(120);
-        cache = RC.concurrentMap(internal, Ref.WEAK_IDENTITY, Ref.SOFT);
+//        ConcurrentMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>> internal 
+//                = new ConcurrentHashMap<Referrer<ProtectionDomain>,Referrer<PermissionCollection>>(120);
+//        cache = RC.concurrentMap(internal, Ref.WEAK_IDENTITY, Ref.SOFT);
         loggable = logger.isLoggable(Level.FINEST);
 	grantLock = new Object();
 	revokePermission = new PolicyPermission("REVOKE");
@@ -302,16 +312,17 @@ public class DynamicPolicyProvider exten
             basePolicyIsDynamic = false;
             revokeable = revoke.equals(tRue);
         }
-        if (basePolicy instanceof RemotePolicy){
-            basePolicyIsRemote = true;
-        } else {
-            basePolicyIsRemote = false;
-        }
-        transactionID = 0;
-        ReadWriteLock rwl = new ReentrantReadWriteLock();
-        transactionWriteLock = rwl.writeLock();
-        transactionReadLock = rwl.readLock();
-        ensureDependenciesResolved();
+        basePolicyIsRemote = basePolicy instanceof RemotePolicy ?true: false;
+        basePolicyIsConcurrent = basePolicy instanceof ConcurrentPolicy 
+                ? ((ConcurrentPolicy) basePolicy).isConcurrent() : false;
+//        transactionID = 0;
+//        ReadWriteLock rwl = new ReentrantReadWriteLock();
+//        transactionWriteLock = rwl.writeLock();
+//        transactionReadLock = rwl.readLock();
+        policyDomain = getClass().getProtectionDomain();
+        policyPermissions = basePolicy.getPermissions(policyDomain);
+        policyPermissions.setReadOnly();
+//        ensureDependenciesResolved();
     }
     
     /**
@@ -328,9 +339,9 @@ public class DynamicPolicyProvider exten
         dynamicPolicyGrants = CollectionsConcurrent.multiReadCollection(
                 new ArrayList<PermissionGrant>(120));
 	remotePolicyGrants = new PermissionGrant[0];
-        ConcurrentMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>> internal 
-                = new ConcurrentHashMap<Referrer<ProtectionDomain>,Referrer<PermissionCollection>>(120);
-        cache = RC.concurrentMap(internal, Ref.WEAK_IDENTITY, Ref.SOFT);
+//        ConcurrentMap<Referrer<ProtectionDomain>, Referrer<PermissionCollection>> internal 
+//                = new ConcurrentHashMap<Referrer<ProtectionDomain>,Referrer<PermissionCollection>>(120);
+//        cache = RC.concurrentMap(internal, Ref.WEAK_IDENTITY, Ref.SOFT);
         loggable = logger.isLoggable(Level.FINEST);
 	grantLock = new Object();
 	revokePermission = new PolicyPermission("REVOKE");
@@ -349,16 +360,17 @@ public class DynamicPolicyProvider exten
             basePolicyIsDynamic = false;
             revokeable = true;
         }
-        if (basePolicy instanceof RemotePolicy){
-            basePolicyIsRemote = true;
-        } else {
-            basePolicyIsRemote = false;
-        }
-        transactionID = 0;
-        ReadWriteLock rwl = new ReentrantReadWriteLock();
-        transactionWriteLock = rwl.writeLock();
-        transactionReadLock = rwl.readLock();
-        ensureDependenciesResolved();
+        basePolicyIsRemote = basePolicy instanceof RemotePolicy ?true: false;
+        basePolicyIsConcurrent = basePolicy instanceof ConcurrentPolicy 
+                ? ((ConcurrentPolicy) basePolicy).isConcurrent() : false;
+//        transactionID = 0;
+//        ReadWriteLock rwl = new ReentrantReadWriteLock();
+//        transactionWriteLock = rwl.writeLock();
+//        transactionReadLock = rwl.readLock();
+        policyDomain = getClass().getProtectionDomain();
+        policyPermissions = basePolicy.getPermissions(policyDomain);
+        policyPermissions.setReadOnly();
+//        ensureDependenciesResolved();
     }
 
     /**
@@ -412,19 +424,71 @@ Work Around 	
 
 Put the policy providers and all referenced classes in the bootstrap class loader.
      */
-    private void ensureDependenciesResolved() {
-        // Investigate bug 4911907, do we need to do anything?
-        // From the work around above, we might not need to do anything.
-        // But these actions prevent the JVM from delaying classloading
-        // of required classes.
-        ProtectionDomain own = this.getClass().getProtectionDomain();
-        implies(own, new AllPermission());
-        new GrantPermission(new UmbrellaGrantPermission());
-    }
+//    private void ensureDependenciesResolved() {
+//        // Investigate bug 4911907, do we need to do anything?
+//        // From the work around above, we might not need to do anything.
+//        // But these actions prevent the JVM from delaying classloading
+//        // of required classes.
+//        ProtectionDomain own = this.getClass().getProtectionDomain();
+//        implies(own, new AllPermission());
+//        new GrantPermission(new UmbrellaGrantPermission());
+//    }
 
     public boolean revokeSupported() {
         return revokeable;
     }
+    
+    private PermissionCollection convert(NavigableSet<Permission> permissions){
+        PermissionCollection pc = new Permissions();
+        // The descending iterator is for SocketPermission.
+        Iterator<Permission> it = permissions.descendingIterator();
+        while (it.hasNext()) {
+            pc.add(it.next());
+        }
+        return pc;
+    }
+    
+    private NavigableSet<Permission> processGrants(PermissionGrant[] grant, Class permClass, boolean stopIfAll)
+    {
+        NavigableSet<Permission> set = new TreeSet<Permission>(comparator);
+        int l = grant.length;
+        if (permClass == null)
+        {
+            for (int i = 0; i < l; i++)
+            {
+                if ( stopIfAll && grant[i].isPrivileged()){
+                    set.add(ALL_PERMISSION);
+                    return set;
+                }
+                Iterator<Permission> it = grant[i].getPermissions().iterator();
+                while (it.hasNext())
+                {
+                    Permission p = it.next();
+                    set.add(p);
+                }
+            }
+        } 
+        else 
+        {
+            for (int i = 0; i < l; i++)
+            {
+                if ( stopIfAll && grant[i].isPrivileged()){
+                    set.add(ALL_PERMISSION);
+                    return set;
+                }
+                Iterator<Permission> it = grant[i].getPermissions().iterator();
+                while (it.hasNext())
+                {
+                    Permission p = it.next();
+                    if (permClass.isInstance(p)|| p instanceof UnresolvedPermission) 
+                    {
+                        set.add(p);
+                    }
+                }
+            }
+        }
+        return set;
+    }
 
     @Override
     public PermissionCollection getPermissions(CodeSource codesource) {
@@ -433,15 +497,20 @@ Put the policy providers and all referen
 	 * by a ProtectionDomain.  In this case during construction of a
 	 * ProtectionDomain.  Static Permissions are irrevocable.
 	 */ 
-        if (revokeable == true){
-            return basePolicy.getPermissions(codesource);
-        }
-        PermissionCollection pc = basePolicy.getPermissions(codesource);
-        PermissionCollection permissions = new Permissions();
-        Enumeration<Permission> enu = pc.elements();
-        while (enu.hasMoreElements()){
-            permissions.add(enu.nextElement());
+        NavigableSet<Permission> permissions = null;
+        if (!basePolicyIsConcurrent || codesource == null) {
+            permissions = new TreeSet<Permission>(comparator);
+            PermissionCollection pc = basePolicy.getPermissions(codesource);
+            Enumeration<Permission> enu = pc.elements();
+            while (enu.hasMoreElements()){
+                permissions.add(enu.nextElement());
+            }
+        }else{
+            ProtectionDomain pd = new ProtectionDomain(codesource, null);
+            PermissionGrant [] grants = ((ConcurrentPolicy) basePolicy).getPermissionGrants(pd);
+            permissions = processGrants(grants, null, true);
         }
+        if (revokeable == true) return convert(permissions);
         Iterator<PermissionGrant> dynamicGrants = dynamicPolicyGrants.iterator();
         while (dynamicGrants.hasNext()){
             PermissionGrant p = dynamicGrants.next();
@@ -454,30 +523,28 @@ Put the policy providers and all referen
                 }
 	    }
         }
-	return permissions;
+	return convert(permissions);
     }
 
     @Override
     public PermissionCollection getPermissions(ProtectionDomain domain) {
-	/* It is extremely important that dynamic grant's are not returned,
-	 * to prevent them becoming part of the static permissions held
-	 * by a ProtectionDomain. In this case during the merge operation
-	 * performed by the ProtectionDomain.
-	 * 
-	 * Dynamic granted Permission's are wrapped in PolicyPermission's
-	 * so ProtectionDomain's print debugging information when toString() 
-         * is called. PolicyPermission doesn't imply anything, it
-         * implements toString() for debugging purposes, it's just a
-	 * container.
-         * 
-         * UPDATE: I've since found PolicyPermission's to be unnecessary, the 
-         * ProtectionDomain only temporarily merges the permissions for toString()
-         * not policy decisions.
-         * 
-         * TODO: Remove PolicyPermission's
+        if (domain == policyDomain) return policyPermissions;
+	/* Note: we can return revokeable permissions, the  ProtectionDomain
+         * only temporarily merges the permissions for toString(), not implies.
 	 */
-        PermissionCollection pc = basePolicy.getPermissions(domain);
-        pc = PolicyUtils.asConcurrent(pc);
+        NavigableSet<Permission> permissions = null;
+        if (!basePolicyIsConcurrent) {
+            permissions = new TreeSet<Permission>(comparator);
+            PermissionCollection pc = basePolicy.getPermissions(domain);
+            Enumeration<Permission> enu = pc.elements();
+            while (enu.hasMoreElements()){
+                permissions.add(enu.nextElement());
+            }
+        }else{
+            PermissionGrant [] grants = 
+                    ((ConcurrentPolicy) basePolicy).getPermissionGrants(domain);
+            permissions = processGrants(grants, null, false);
+        }
 	PermissionGrant [] grantsRefCopy = remotePolicyGrants; // Interim updates not seen.
 	int l = grantsRefCopy.length;
 	for ( int i = 0; i < l; i++ ){
@@ -485,7 +552,7 @@ Put the policy providers and all referen
 		Collection<Permission> perms = grantsRefCopy[i].getPermissions();
 		Iterator<Permission> it = perms.iterator();
                 while (it.hasNext()){
-                    pc.add(it.next());
+                    permissions.add(it.next());
                 }
 	    }
 	}
@@ -497,11 +564,11 @@ Put the policy providers and all referen
                 Collection<Permission> perms = p.getPermissions();
                 Iterator<Permission> it = perms.iterator();
                 while (it.hasNext()){
-                    pc.add(it.next());
+                    permissions.add(it.next());
                 }
 	    }
         }
-	return pc;	
+	return convert(permissions);	
     }
     
     /* River-26 Mark Brouwer suggested making UmbrellaPermission's expandable
@@ -513,6 +580,7 @@ Put the policy providers and all referen
 
     @Override
     public boolean implies(ProtectionDomain domain, Permission permission) {
+        if (domain == policyDomain) return policyPermissions.implies(permission);
         if (basePolicyIsDynamic || basePolicyIsRemote){
             // Total delegation revoke supported only by underlying policy.
             if (basePolicy.implies(domain, permission)) return true;
@@ -520,28 +588,28 @@ Put the policy providers and all referen
 	if (permission == null) throw new NullPointerException("permission not allowed to be null");
         // First check our cache if the basePolicy is not dynamic.
         
-        PermissionCollection pc = domain != null? cache.get(domain): null;
-        if ( pc != null ) {
-            /* Out of date cache is cleared and only updated with the latest 
-             * grants don't bother retrieving it again */
-            if ( pc.implies(permission) ) return true;          
-        } 
-        Thread thread = Thread.currentThread();
-        if (thread.isInterrupted()) return false;
+//        PermissionCollection permissions = domain != null? cache.get(domain): null;
+//        if ( permissions != null ) {
+//            /* Out of date cache is cleared and only updated with the latest 
+//             * grants don't bother retrieving it again */
+//            if ( permissions.implies(permission) ) return true;          
+//        } 
+//        Thread thread = Thread.currentThread();
+//        if (thread.isInterrupted()) return false;
         /* Do not call implies on the base Policy, if
          * there are UnresolvedPermission's that are undergoing resolution
          * while another Permission within that collection is already
          * resolved, the Enumeration will cause a ConcurrentModificationException.
          */ 
-        int currentTransactionID;
-        PermissionCollection bpc = null;
-        transactionReadLock.lock();
-        try {
-            currentTransactionID = transactionID;
-            bpc = basePolicy.getPermissions(domain);
-        }finally{
-            transactionReadLock.unlock();
-        }
+//        int currentTransactionID;
+//        PermissionCollection bpc = null;
+//        transactionReadLock.lock();
+//        try {
+//            currentTransactionID = transactionID;
+//            bpc = basePolicy.getPermissions(domain);
+//        }finally{
+//            transactionReadLock.unlock();
+//        }
         
         /* Be mindful of static Permissions held by the 
          * ProtectionDomain, a Permission may be implied by the 
@@ -563,14 +631,37 @@ Put the policy providers and all referen
        /* Don't use the underlying policy permission collection otherwise
         * we can leak grants in to the underlying policy from our cache,
         * this could then be merged into the PermissionDomain's permission
-        * cache negating the possiblity of revoking the permission.  This
-        * PolicyUtils method defensively copies or creates new if null.
+        * cache negating the possiblity of revoking the permission. 
         */
-        pc = PolicyUtils.asConcurrent(bpc);
+//        permissions = PolicyUtils.asConcurrent(bpc);
         /* Don't place it in the cache half finished or check it yet since
          * mutations are blocking */
+        NavigableSet<Permission> permissions = null; // Keep as small as possible.
+        /* If GrantPermission is being requested, we must get all Permissions
+         * and add them to the underlying collection.
+         * 
+         */
+        Class permClass = permission instanceof GrantPermission ? null : permission.getClass();
+        if (!basePolicyIsConcurrent) {
+            permissions = new TreeSet<Permission>(comparator);
+            PermissionCollection pc = basePolicy.getPermissions(domain);
+            Enumeration<Permission> enu = pc.elements();
+            while (enu.hasMoreElements()){
+                Permission p = enu.nextElement();
+                if (p instanceof AllPermission) return true; // Return early.
+                if ( permClass == null){
+                    permissions.add(p);
+                } else if ( permClass.isInstance(permission) || permission instanceof UnresolvedPermission){
+                    permissions.add(p);
+                }
+            }
+        }else{
+            PermissionGrant [] grants = ((ConcurrentPolicy) basePolicy).getPermissionGrants(domain);
+            permissions = processGrants(grants, permClass, true);
+            if (permissions.contains(ALL_PERMISSION)) return true;
+        }
 	PermissionGrant[] grantsRefCopy = remotePolicyGrants; // In case the grants volatile reference is updated.       
-        if (thread.isInterrupted()) return false;
+//        if (thread.isInterrupted()) return false;
 	int l = grantsRefCopy.length;
 	for ( int i = 0; i < l; i++){
 	    if (grantsRefCopy[i].implies(domain)) {
@@ -578,11 +669,16 @@ Put the policy providers and all referen
 		Collection<Permission> perms = grantsRefCopy[i].getPermissions();
 		Iterator<Permission> it = perms.iterator();
                 while (it.hasNext()){
-                    pc.add(it.next());
+                    Permission p = it.next();
+                    if ( permClass == null){
+                        permissions.add(p);
+                    } else if ( permClass.isInstance(permission) || permission instanceof UnresolvedPermission){
+                        permissions.add(p);
+                    }
                 }
 	    }
 	}
-        if (thread.isInterrupted()) return false;
+//        if (thread.isInterrupted()) return false;
         Iterator<PermissionGrant> grants = dynamicPolicyGrants.iterator();
         while (grants.hasNext()){
             PermissionGrant g = grants.next();
@@ -590,23 +686,30 @@ Put the policy providers and all referen
                 Collection<Permission> perms = g.getPermissions();
                 Iterator<Permission> it = perms.iterator();
                 while (it.hasNext()){
-                    pc.add(it.next());
+                    Permission p = it.next();
+                    if ( permClass == null){
+                        permissions.add(p);
+                    } else if ( permClass.isInstance(permission) || permission instanceof UnresolvedPermission){
+                        permissions.add(p);
+                    }
                 }
             }
         }
-        if (thread.isInterrupted()) return false;
+//        if (thread.isInterrupted()) return false;
         // We have added dynamic grants, lets expand any UmbrellaGrant's
-        expandUmbrella(pc);
-        if (domain != null) {
-            if (transactionReadLock.tryLock()){
-                try { 
-                    if (transactionID == currentTransactionID) 
-                        cache.putIfAbsent(domain, pc);
-                }finally {
-                    transactionReadLock.unlock();
-                }
-            }
-        }
+        
+        PermissionCollection pc = convert(permissions);
+        if (permission instanceof GrantPermission) expandUmbrella(pc);
+//        if (domain != null) {
+//            if (transactionReadLock.tryLock()){
+//                try { 
+//                    if (transactionID == currentTransactionID) 
+//                        cache.putIfAbsent(domain, permissions);
+//                }finally {
+//                    transactionReadLock.unlock();
+//                }
+//            }
+//        }
         return pc.implies(permission);
     }
     
@@ -622,13 +725,13 @@ Put the policy providers and all referen
     
     @SuppressWarnings("unchecked")
     public void refresh() {
-        transactionWriteLock.lock();
-        try {
+//        transactionWriteLock.lock();
+//        try {
             basePolicy.refresh();
-            transactionID++;
-        }finally{
-            transactionWriteLock.unlock();
-        }
+//            transactionID++;
+//        }finally{
+//            transactionWriteLock.unlock();
+//        }
         // Clean up any void dynamic grants.
         Collection<PermissionGrant> remove = new ArrayList<PermissionGrant>(40);
 	Iterator<PermissionGrant> i = dynamicPolicyGrants.iterator();
@@ -640,13 +743,13 @@ Put the policy providers and all referen
         }
         dynamicPolicyGrants.removeAll(remove);
         // Increment transaction ID after cache clear, 
-        transactionWriteLock.lock();
-        try {
-            cache.clear();
-            transactionID++;
-        }finally{
-            transactionWriteLock.unlock();
-        }
+//        transactionWriteLock.lock();
+//        try {
+//            cache.clear();
+//            transactionID++;
+//        }finally{
+//            transactionWriteLock.unlock();
+//        }
         // Don't bother removing void from the remotePolicy, it get's replaced anyway.
         // Policy file based grant's don't become void, only dynamic grant's
         // to ProtectionDomain or ClassLoader.
@@ -692,13 +795,13 @@ Put the policy providers and all referen
 	// This grant is new, in the grantCache and we trust it.
 	dynamicPolicyGrants.add(pe);
         // Increment transaction ID after cache clear, 
-        transactionWriteLock.lock();
-        try {
-            cache.clear();
-            transactionID++;
-        }finally{
-            transactionWriteLock.unlock();
-        }
+//        transactionWriteLock.lock();
+//        try {
+//            cache.clear();
+//            transactionID++;
+//        }finally{
+//            transactionWriteLock.unlock();
+//        }
 //	if (loggable){
 //	    logger.log(Level.FINEST, "Granting: {0}", pe.toString());
 //	}
@@ -760,13 +863,13 @@ Put the policy providers and all referen
         // ProtectionDomains a ClassLoader references, so we must clear the
         // cache.
         // Increment transaction ID after cache clear.
-        transactionWriteLock.lock();
-        try {
-            cache.clear();
-            transactionID++;
-        }finally{
-            transactionWriteLock.unlock();
-        }
+//        transactionWriteLock.lock();
+//        try {
+//            cache.clear();
+//            transactionID++;
+//        }finally{
+//            transactionWriteLock.unlock();
+//        }
         
         SecurityManager sm = System.getSecurityManager();
         if (sm instanceof DelegateSecurityManager) {
@@ -890,13 +993,13 @@ Put the policy providers and all referen
 	    PermissionGrant[] updated = new PermissionGrant[holder.size()];
 	    remotePolicyGrants = holder.toArray(updated);
 	}
-        transactionWriteLock.lock();
-        try {
-            cache.clear();
-            transactionID++;
-        }finally{
-            transactionWriteLock.unlock();
-        }
+//        transactionWriteLock.lock();
+//        try {
+//            cache.clear();
+//            transactionID++;
+//        }finally{
+//            transactionWriteLock.unlock();
+//        }
         Collection<PermissionGrant> oldGrants = new HashSet<PermissionGrant>(old.length);
         oldGrants.addAll(Arrays.asList(old));
         oldGrants.removeAll(holder);

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/DelegateCombinerSecurityManager.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/DelegateCombinerSecurityManager.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/DelegateCombinerSecurityManager.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/DelegateCombinerSecurityManager.java Mon Jan  9 13:12:52 2012
@@ -20,7 +20,6 @@ package org.apache.river.api.security;
 
 import java.security.AccessControlContext;
 import java.security.AccessController;
-import java.security.AllPermission;
 import java.security.DomainCombiner;
 import java.security.Guard;
 import java.security.Permission;
@@ -33,15 +32,17 @@ import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.NavigableSet;
 import java.util.Set;
-import java.util.TreeSet;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Future;
+import java.util.concurrent.Executor;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.RunnableFuture;
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
@@ -50,7 +51,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import net.jini.security.PermissionComparator;
 import org.apache.river.api.delegates.DelegatePermission;
-import org.apache.river.impl.util.CollectionsConcurrent;
 import org.apache.river.impl.util.RC;
 import org.apache.river.impl.util.Ref;
 import org.apache.river.impl.util.Referrer;
@@ -63,6 +63,9 @@ import org.apache.river.impl.util.Referr
  * This SecurityManager should be tuned for garbage collection for a large
  * young generation heap, since many young objects are created and discarded.
  * 
+ * It is recommended that this SecurityManager be installed from the command
+ * line in order to load as early as possible.
+ * 
  * @author Peter Firmstone
  */
 public class DelegateCombinerSecurityManager 
@@ -71,29 +74,36 @@ extends SecurityManager implements Deleg
     private final DomainCombiner dc;
     // Cache of optimised Delegate AccessControlContext's
     private final ConcurrentMap<AccessControlContext, AccessControlContext> contextCache;
-    private final ConcurrentMap<AccessControlContext, Set<Permission>> checked;
+    private final ConcurrentMap<AccessControlContext, NavigableSet<Permission>> checked;
     private final Guard g;
     private final Action action;
-    private final ExecutorService executor;
+    private final Executor executor;
     private final Comparator<Referrer<Permission>> permCompare;
-    private final AccessControlContext SMContext;
+    private final AccessControlContext SMConstructorContext;
+    private final AccessControlContext SMPrivilegedContext;
+    private final Guard createAccPerm;
+    private final Logger logger;
     
     public DelegateCombinerSecurityManager(){
         super();
         // Get context before this becomes a SecurityManager.
         // super() checked the permission to create a SecurityManager.
-        SMContext = AccessController.getContext();
+        SMConstructorContext = AccessController.getContext();
+        ProtectionDomain [] context = new ProtectionDomain[1];
+        context[0] = this.getClass().getProtectionDomain();
+        SMPrivilegedContext = new AccessControlContext(context);
         dc = new DelegateDomainCombiner();
         ConcurrentMap<Referrer<AccessControlContext>, 
                 Referrer<AccessControlContext>> internal = 
                 new ConcurrentHashMap<Referrer<AccessControlContext>, 
                 Referrer<AccessControlContext>>(100);
         contextCache = RC.concurrentMap(internal, Ref.SOFT, Ref.STRONG);
-        ConcurrentMap<Referrer<AccessControlContext>, Referrer<Set<Permission>>> refmap 
+        ConcurrentMap<Referrer<AccessControlContext>, Referrer<NavigableSet<Permission>>> refmap 
                 = new ConcurrentHashMap<Referrer<AccessControlContext>, 
-                Referrer<Set<Permission>>>(100);
+                Referrer<NavigableSet<Permission>>>(100);
         checked = RC.concurrentMap(refmap, Ref.SOFT, Ref.STRONG);
         g = new SecurityPermission("getPolicy");
+        createAccPerm = new SecurityPermission("createAccessControlContext");
         action = new Action();
         // Make this a tunable property.
         double blocking_coefficient = 0.6; // 0 CPU intensive to 0.9 IO intensive
@@ -111,8 +121,10 @@ extends SecurityManager implements Deleg
                 TimeUnit.SECONDS, new SynchronousQueue<Runnable>(), 
                 new ThreadPoolExecutor.CallerRunsPolicy());
         permCompare = RC.comparator(new PermissionComparator());
-        // Refresh the policy, in case it hasn't parsed all policy files.
-        java.security.Policy.getPolicy().refresh();
+        logger = Logger.getLogger(DelegateCombinerSecurityManager.class.getName());
+        // Get the policy & refresh, in case it hasn't been initialized.
+        java.security.Policy.getPolicy().refresh(); // Investigate some other way of ensuring the policy is instantiated.
+        // Bug ID: 7093090 Reduce synchronization in java.security.Policy.getPolicyNoCheck
     }
     
     public void checkPermission(Permission perm) throws SecurityException {
@@ -123,45 +135,73 @@ extends SecurityManager implements Deleg
     public void checkPermission(Permission perm, Object context) throws SecurityException {
 	if (!(context instanceof AccessControlContext)) throw new SecurityException();
 	if (perm == null ) throw new NullPointerException("Permission Collection null");
-        if (SMContext.equals(context)) return; // prevents endless loop in debug.
+        /* The next line speeds up permission checks related to this SecurityManager. */
+        if ( SMPrivilegedContext.equals(context) || SMConstructorContext.equals(context)) return; // prevents endless loop in debug.
         AccessControlContext executionContext = (AccessControlContext) context;
         // Checks if Permission has already been checked for this context.
-        Set<Permission> checkedPerms = checked.get(executionContext);
+        NavigableSet<Permission> checkedPerms = checked.get(executionContext);
         if (checkedPerms == null){
-            Set<Referrer<Permission>> internal = 
-                    CollectionsConcurrent.multiReadSet(new TreeSet<Referrer<Permission>>(permCompare));
-            checkedPerms = RC.set(internal, Ref.SOFT);
-            Set<Permission> existed = checked.putIfAbsent(executionContext, checkedPerms);
+            /* A ConcurrentSkipListSet is used to avoid blocking during
+             * removal operations that occur while the garbage collector
+             * recovers softly reachable memory.  Since this happens while
+             * the jvm's under stress, it's important that permission checks
+             * continue to perform well.
+             * 
+             * Although I considered a multi read, single write Set, I wanted
+             * to avoid blocking under stress, caused as a result
+             * of garbage collection.
+             * 
+             * The Reference Collection that encapsulates the ConcurrentSkipListSet
+             * uses a ReentrantLock.tryLock() guard the ReferenceQueue used
+             * to remove objects from the Set.  This allows other threads to
+             * proceed during object removal.  Only one thread is given access
+             * to the ReferenceQueue, the unlucky caller thread performs garbage
+             * removal from the Set before accessing the Set for its original
+             * purpose.
+             */
+            NavigableSet<Referrer<Permission>> internal = 
+                    new ConcurrentSkipListSet<Referrer<Permission>>(permCompare);
+            checkedPerms = RC.navigableSet(internal, Ref.SOFT);
+            NavigableSet<Permission> existed = checked.putIfAbsent(executionContext, checkedPerms);
             if (existed != null) checkedPerms = existed;
         }
         if (checkedPerms.contains(perm)) return; // don't need to check again.
-        // This is an expensive operation, so we cache the created AccessControlContext.
+        // Cache the created AccessControlContext.
         AccessControlContext delegateContext = contextCache.get(executionContext);
-        if (delegateContext == null) {
-            final AccessControlContext finalExecutionContext = executionContext;
-            // Create a new AccessControlContext with the DelegateDomainCombiner
-            // we don't need to preserve the Subject accross the call
-            // we have sufficient privilege.
-            delegateContext = AccessController.doPrivileged( 
-                new PrivilegedAction<AccessControlContext>(){
-                    public AccessControlContext run() {
-                        return new AccessControlContext(finalExecutionContext, dc);
+        if (delegateContext == null ) {
+            /* It is not possible to "create" a delegate AccessControlContext
+             * for checking permission to create an AccessControlContext
+             * with DomainCombiner, it creates a recursive loop.  In this
+             * case just use the executionContext.
+             */
+            if (createAccPerm.equals(perm)){
+                delegateContext = executionContext;
+            } else {
+                final AccessControlContext finalExecutionContext = executionContext;
+                // Create a new AccessControlContext with the DelegateDomainCombiner
+                // we don't need to preserve the Subject accross the call
+                // we have sufficient privilege.
+                delegateContext = AccessController.doPrivileged( 
+                    new PrivilegedAction<AccessControlContext>(){
+                        public AccessControlContext run() {
+                            return new AccessControlContext(finalExecutionContext, dc);
+                        }
                     }
-                }
-            );
-            // Optimise the delegateContext, this runs the DelegateDomainCombiner
-            // and returns the AccessControlContext.
-            // This is a mutator method, the delegateContext returned
-            // is actually the same object passed in, after it is
-            // mutated, but just in case that changes in future we
-            // return it.
-            delegateContext = AccessController.doPrivileged(action, delegateContext);
-            contextCache.putIfAbsent(executionContext, delegateContext);
-            // Above putIfAbsent: It doesn't matter if it already existed,
-            // the context we have is valid to perform a permissionCheck.
+                );
+                // Optimise the delegateContext, this runs the DelegateDomainCombiner
+                // and returns the AccessControlContext.
+                // This is a mutator method, the delegateContext returned
+                // is actually the same object passed in, after it is
+                // mutated, but just in case that changes in future we
+                // return it.
+                delegateContext = AccessController.doPrivileged(action, delegateContext);
+                contextCache.putIfAbsent(executionContext, delegateContext);
+                // Above putIfAbsent: It doesn't matter if it already existed,
+                // the context we have is valid to perform a permissionCheck.
+            }
         }
         // Normal execution, same as SecurityManager.
-        delegateContext.checkPermission(perm);
+        delegateContext.checkPermission(perm); // Throws SecurityException.
         /* It's ok to cache SocketPermission if we use a comparator */
         // If we get to here, no exceptions were thrown, we have permission.
         // Don't cache SocketPermission.
@@ -194,9 +234,9 @@ extends SecurityManager implements Deleg
         while (i.hasNext()){
             classes.add(i.next().getClass());
         }
-        Collection<Set<Permission>> cache = checked.values();
+        Collection<NavigableSet<Permission>> cache = checked.values();
         Collection<Permission> remove = new ArrayList<Permission>(60);
-        Iterator<Set<Permission>> j = cache.iterator();
+        Iterator<NavigableSet<Permission>> j = cache.iterator();
         while (j.hasNext()){
             Set<Permission> s = j.next();
             Iterator<Permission> k = s.iterator();
@@ -224,7 +264,7 @@ extends SecurityManager implements Deleg
         private DelegateDomainCombiner (){
         }
 
-        public ProtectionDomain[] combine(ProtectionDomain[] currentDomains, ProtectionDomain[] assignedDomains) {
+        public ProtectionDomain[] combine(final ProtectionDomain[] currentDomains, final ProtectionDomain[] assignedDomains) {
             /* We're only interested in the assignedDomains, since these
              * are from the Context that the SecurityManager has been asked
              * to check.
@@ -249,8 +289,7 @@ extends SecurityManager implements Deleg
     }
     
     /*
-     * DelegateProtectionDomain is only used briefly on the stack for
-     * a permission check.  The policy will never see it.
+     * DelegateProtectionDomain executes checks on ProtectionDomain's in parallel.
      */
     private class DelegateProtectionDomain extends ProtectionDomain {
         // Context from AccessControlContext.
@@ -260,70 +299,84 @@ extends SecurityManager implements Deleg
             // Use static domain so we don't strong reference to ClassLoader
             // which has a strong reference to ProtectionDomain.
             super(null, null);
-            this.context = context;
+            this.context = context; // Not mutated so don't need to clone.
         }
         
         @Override
         public boolean implies(Permission perm) {
             Thread currentThread = Thread.currentThread();
+            boolean interrupt = Thread.interrupted(); // Clears the interrupt and stores it.
             int l = context.length;
             if ( l < 4 ){ 
                 // We've got at least one priviledged domain that will execute
                 // quickly so process it in this thread.
                 // This also reduces contention on the Executor, which should be saved
                 // for situations where there are a large number of domains.
+                // This also avoids unnecessary overhead when calling AccessController.doPrivileged()
+                // and recursion that might occur
                 for ( int i = 0; i < l; i++ ){
-                    if (! checkPermission(context[i], perm)) return false;
+                    if (! checkPermission(context[i], perm)) {
+                        if (interrupt) currentThread.interrupt();
+                        return false;
+                    }
                 }
+                if (interrupt) currentThread.interrupt();
                 return true;
             }
             CountDownLatch latch = new CountDownLatch(l);
-            List<Future<Boolean>> resultList = null;
-            Collection<Callable<Boolean>> tasks = new ArrayList<Callable<Boolean>>(l);
+            List<RunnableFuture<Boolean>> resultList = new ArrayList<RunnableFuture<Boolean>>(l);
             AtomicBoolean terminated = new AtomicBoolean(false);
             for ( int i = 0; i < l; i++ ){
-                tasks.add(new PermissionCheck(context[i], perm, latch, currentThread, terminated));
+                resultList.add(new FutureTask<Boolean>(
+                    new PermissionCheck(context[i], perm, latch, currentThread, terminated)
+                ));
+            }
+            Iterator<RunnableFuture<Boolean>> it = resultList.iterator();
+            while (it.hasNext()){
+                executor.execute(it.next());
             }
             try {
                 // We can change either call to add a timeout.
-                resultList = executor.invokeAll(tasks);
-                latch.await();
-                Iterator<Future<Boolean>> it = resultList.iterator();
+                latch.await(); // Throws InterruptedException
+                it = resultList.iterator();
                 try {
                     while (it.hasNext()){
-                            Boolean result = it.next().get();
+                            Boolean result = it.next().get(); // Throws InterruptedException
                             if (result.equals(Boolean.FALSE)) {
+                                if (interrupt) currentThread.interrupt();
                                 return false;
                             }
                     }
+                    if (interrupt) currentThread.interrupt();
                     return true;
                 } catch (ExecutionException ex) {
                     // This should never happen, unless a runtime exception occurs.
-                    Logger.getLogger(DelegateCombinerSecurityManager.class.getName()).log(Level.SEVERE, null, ex);
+                    if (logger.isLoggable(Level.SEVERE)) logger.log(Level.SEVERE, null, ex);
                 }
             } catch (InterruptedException ex) {
-                // A task could return false, after the interruption and be
-                // interleaved before this check, it isn't foolproof.  This
-                // cannot be worked around since the exception clears the interrupt
-                // status.
-                // In that case the SecurityException will be thrown and the
-                // interrupt status swallowed.
-                boolean externalInterrupt = false;
-                if (!terminated.get()) externalInterrupt = true;
-                if ( resultList != null ){ // could be interrupted before executor invoking.
-                    Iterator<Future<Boolean>> it = resultList.iterator();
+                // REMIND: Java Memory Model and thread interruption.
+                if (terminated.get()){
+                    /* Interrupted by a task returning false */
+                    it = resultList.iterator();
                     while (it.hasNext()){
                         it.next().cancel(true);
                     }
+                    if (interrupt) currentThread.interrupt();
+                    return false;
+                }                 
+                // We've been externally interrupted, during execution.
+                // Do this the slow way!
+                if (logger.isLoggable(Level.FINEST)) logger.log(Level.FINEST, "External Interruption", ex);
+                for ( int i = 0; i < l; i++ ){
+                    if (! checkPermission(context[i], perm)) {
+                        currentThread.interrupt(); // restore external interrupt.
+                        return false;
+                    }
                 }
-                Logger.getLogger(DelegateCombinerSecurityManager.class.getName()).log(Level.FINEST, null, ex);
-                // Task interruption is normal, it just means one task returned false.
-                // Swallow the interrupt, unless externally interrupted.
-                if (externalInterrupt){
-                    // Restore external interrupt.
-                    currentThread.interrupt();
-                }
+                currentThread.interrupt(); // restore external interrupt.
+                return true;
             }
+            if (interrupt) currentThread.interrupt();
             return false;
         }
         
@@ -352,30 +405,46 @@ extends SecurityManager implements Deleg
         private final ProtectionDomain pd;
         private final Permission p;
         private final CountDownLatch latch;
-        private final Thread caller;
+        private final Thread callThread;
         private final AtomicBoolean terminated;
+        private final ClassLoader contextClassLoader; // Required for AggregatePolicyProvider.
         
         PermissionCheck(ProtectionDomain pd, Permission p, CountDownLatch c, Thread caller, AtomicBoolean terminated){
             if (pd == null || p == null) throw new NullPointerException();
             this.pd = pd;
             this.p = p;
             latch = c;
-            this.caller = caller;
+            this.callThread = caller;
             this.terminated = terminated;
+            contextClassLoader = AccessController.doPrivileged(new PrivilegedAction<ClassLoader>(){
+                public ClassLoader run() {
+                    return callThread.getContextClassLoader();
+                }
+            });
         }
 
         public Boolean call() throws Exception {
-            boolean result = false;
-            result = checkPermission(pd, p);
+            // Required for AggregatePolicyProvider.
+            AccessController.doPrivileged(new PrivilegedAction(){
+                public Object run() {
+                    Thread.currentThread().setContextClassLoader(contextClassLoader);
+                    return null;
+                }
+            });
+            boolean result = checkPermission(pd, p);
             // If we need to check for any Permission that we have substituted
             // for a standard jvm permission, getPermissions and convert,
             // then test here for static ProtectionDomain's.
-            if ( result == false ) {
-                // This only allows one task to terminate the caller,
-                // preventing late terminating tasks from resetting the interrupt.
+            if (!result) {
+                // This only allows one task to terminate the caller, provided
+                // that no external interruption has occurred.  The interrupt
+                // status is cleared by the InterruptedException,
+                // preventing late terminating tasks from re-interrupting
+                // during cancellation initiated by the caller.
                 // The first task to return false initiates early termination
                 // of the other tasks.
-                if (terminated.compareAndSet(false, true)) caller.interrupt();
+                // Is it ok to interrupt our own thread if under load.
+                if (terminated.compareAndSet(callThread.isInterrupted(), true)) callThread.interrupt(); // Not atomic
             }
             latch.countDown();
             return result;
@@ -384,9 +453,8 @@ extends SecurityManager implements Deleg
     }
     
     static boolean checkPermission(ProtectionDomain pd, Permission p){
-        boolean result = false;
-        result = pd.implies(p);
-        if (result == false && p instanceof DelegatePermission ){
+        boolean result = pd.implies(p);
+        if (!result && p instanceof DelegatePermission ){
             Permission candidate = ((DelegatePermission)p).getPermission();
             result = pd.implies(candidate);
         }

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrant.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrant.java Mon Jan  9 13:12:52 2012
@@ -50,6 +50,13 @@ public interface PermissionGrant {
      * @return
      */
     boolean inverse();
+    
+    /**
+     * Optimisation for AllPermission.
+     * 
+     * @return true - if PermissionGrant contains AllPermission.
+     */
+    boolean isPrivileged();
 
     /**
      * A DynamicPolicy implementation can use a PermissionGrant as a container

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrantBuilderImp.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrantBuilderImp.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrantBuilderImp.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PermissionGrantBuilderImp.java Mon Jan  9 13:12:52 2012
@@ -278,6 +278,11 @@ class PermissionGrantBuilderImp extends 
         public boolean inverse() {
             return false;
         }
+
+        
+        public boolean isPrivileged() {
+            return false;
+        }
         
     }
 }

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/PrincipalGrant.java Mon Jan  9 13:12:52 2012
@@ -25,6 +25,7 @@ import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.security.AllPermission;
 import java.security.CodeSigner;
 import java.security.CodeSource;
 import java.security.Permission;
@@ -54,6 +55,7 @@ class PrincipalGrant implements Permissi
     private final int hashCode;
     private final Set<Permission> perms;
     private final boolean inverse;
+    private final boolean privileged;
     @SuppressWarnings("unchecked")
     PrincipalGrant(Principal[] pals, Permission[] perm, boolean inverse){
         if ( pals != null ){
@@ -65,11 +67,19 @@ class PrincipalGrant implements Permissi
         }
         if (perm == null || perm.length == 0) {
             this.perms = Collections.emptySet();
+            privileged = false;
         }else{
             // PermissionComparator is used to avoid broken hashCode and equals
 	    Set<Permission> perms = new TreeSet<Permission>(new PermissionComparator());
-            perms.addAll(Arrays.asList(perm));
+//            Set<Permission> perms = new HashSet<Permission>();
+            boolean privileged = false;
+            int l = perm.length;
+            for (int i = 0; i < l; i++){
+                perms.add(perm[i]);
+                if (perm[i] instanceof AllPermission) privileged = true;
+            }
 	    this.perms = Collections.unmodifiableSet(perms);
+            this.privileged = privileged;
         }
         int hash = 5;
         hash = 97 * hash + (this.pals != null ? this.pals.hashCode() : 0);
@@ -77,6 +87,7 @@ class PrincipalGrant implements Permissi
         while (i.hasNext()){
             Permission p = i.next();
             if (p != null){
+                // REMIND: hash might not be unique for UnresolvedPermission.
                 Class c = p.getClass();
                 String name = p.getName();
                 String actions = p.getActions();
@@ -267,4 +278,9 @@ class PrincipalGrant implements Permissi
     public boolean inverse() {
         return inverse;
     }
+
+    @Override
+    public boolean isPrivileged() {
+        return privileged;
+    }
 }

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/api/security/URIGrant.java Mon Jan  9 13:12:52 2012
@@ -112,15 +112,15 @@ class URIGrant extends CertificateGrant 
         if ( !implies(p)) return false;
         // sun.security.provider.PolicyFile compatibility for null CodeSource is false.
         // see com.sun.jini.test.spec.policyprovider.dynamicPolicyProvider.GrantPrincipal test.
-        if (codeSource == null)  return false;
-        URL url = codeSource.getLocation();
-        if (url == null) return false;
-        if (location.isEmpty()) return true;
+        if (codeSource == null)  return false; // Null CodeSource is not implied.
+        if (location.isEmpty()) return true; // But CodeSource with null URL is implied, if this location is empty.
         int l = location.size();
         URI[] uris = location.toArray(new URI[l]);
         for (int i = 0; i<l ; i++ ){
             if (uris[i] == null) return true;
         }
+        URL url = codeSource.getLocation();
+        if (url == null ) return false;
         URI implied = null;
         try {
             implied = AccessController.doPrivileged(new NormaliseURLAction(url));

Modified: river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/impl/security/policy/util/DefaultPolicyParser.java
URL: http://svn.apache.org/viewvc/river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/impl/security/policy/util/DefaultPolicyParser.java?rev=1229137&r1=1229136&r2=1229137&view=diff
==============================================================================
--- river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/impl/security/policy/util/DefaultPolicyParser.java (original)
+++ river/jtsk/skunk/peterConcurrentPolicy/src/org/apache/river/impl/security/policy/util/DefaultPolicyParser.java Mon Jan  9 13:12:52 2012
@@ -148,7 +148,8 @@ public class DefaultPolicyParser impleme
             }
             catch (Exception e) {
                 if ( e instanceof SecurityException ) throw (SecurityException) e;
-                e.printStackTrace(System.out);
+                System.err.println("Problem parsing policy: "+ location 
+                        + "\n" + e);
             }
         }
         
@@ -190,6 +191,7 @@ public class DefaultPolicyParser impleme
      */
     protected PermissionGrant resolveGrant(DefaultPolicyScanner.GrantEntry ge,
             KeyStore ks, Properties system, boolean resolve) throws Exception {
+        if ( ge == null ) return null;
         /*
          * Do we return multiple grants or do we allow a codebase array 
          * in a permission grant?
@@ -197,12 +199,12 @@ public class DefaultPolicyParser impleme
          * ANSWER: No we just make a CodeSourceSetGrant, that contains multiple
          * CodeSource.
          */
-        URI codebase = null;
         List<URI> codebases = new ArrayList<URI>(8);
         Certificate[] signers = null;
         Set<Principal> principals = new HashSet<Principal>();
         Set<Permission> permissions = new HashSet<Permission>();
         String cb = ge.getCodebase(null);
+        String signerString = ge.getSigners();
         if ( cb != null ) {
             if ( resolve ) {
                 try {
@@ -218,37 +220,41 @@ public class DefaultPolicyParser impleme
                 codebases.add(new URI(cb));
             }
         }
-        if ( ge.getSigners() != null) {
+        if ( signerString != null) {
             if (resolve) {
-                ge.setSigners(PolicyUtils.expand(ge.getSigners(), system));
+                signerString = PolicyUtils.expand(signerString, system);
             }
-            signers = resolveSigners(ks, ge.getSigners());
+            signers = resolveSigners(ks, signerString);
         }
         if (ge.getPrincipals(null) != null) {
+            String principalName = null;
+            String principalClass = null;
             for (Iterator<PrincipalEntry> iter = ge.getPrincipals(system).iterator(); iter.hasNext();) {
-                DefaultPolicyScanner.PrincipalEntry pe = iter
-                        .next();
+                DefaultPolicyScanner.PrincipalEntry pe = iter.next();
+                principalName = pe.getName();
+                principalClass = pe.getKlass();
                 if (resolve) {
-                    pe.name = PolicyUtils.expand(pe.name, system);
+                    principalName = PolicyUtils.expand(principalName, system);
                 }
-                if (pe.klass == null) {
-                    principals.add(getPrincipalByAlias(ks, pe.name));
+                if (principalClass == null) {
+                    principals.add(getPrincipalByAlias(ks, principalName));
                 } else {
-                    principals.add(new UnresolvedPrincipal(pe.klass, pe.name));
+                    principals.add(new UnresolvedPrincipal(principalClass, principalName));
                 }
             }
         }
-        if (ge.getPermissions() != null) {
-            for (Iterator<PermissionEntry> iter = ge.getPermissions().iterator(); iter.hasNext();) {
-                DefaultPolicyScanner.PermissionEntry pe = iter
-                        .next();
+        Collection<PermissionEntry> pec = ge.getPermissions();
+        if (pec != null) {
+            Iterator<PermissionEntry> iter = pec.iterator();
+            while ( iter.hasNext()) {
+                DefaultPolicyScanner.PermissionEntry pe = iter.next();
                 try {
-                    permissions.add(resolvePermission(pe, ge, ks, system,
-                            resolve));
+                    permissions.add(resolvePermission(pe, ge, ks, system, resolve));
                 }
                 catch (Exception e) {
                     if ( e instanceof SecurityException ) throw (SecurityException) e;
-                    // TODO: log warning
+                    // TODO: log warning.
+                    System.err.println(e);
                 }
             }
         }
@@ -313,33 +319,33 @@ public class DefaultPolicyParser impleme
             DefaultPolicyScanner.PermissionEntry pe,
             DefaultPolicyScanner.GrantEntry ge, KeyStore ks, Properties system,
             boolean resolve) throws Exception {
-        if (pe.name != null) {
-            pe.name = PolicyUtils.expandGeneral(pe.name,
-                    new PermissionExpander().configure(ge, ks));
+        String className = pe.getKlass(), name=pe.getName(), 
+                actions=pe.getActions(), signer=pe.getSigners();
+        if (name != null) {
+            name = PolicyUtils.expandGeneral(name, new PermissionExpander(ge, ks));
         }
         if (resolve) {
-            if (pe.name != null) {
-                pe.name = PolicyUtils.expand(pe.name, system);
+            if (name != null) {
+                name = PolicyUtils.expand(name, system);
             }
-            if (pe.actions != null) {
-                pe.actions = PolicyUtils.expand(pe.actions, system);
+            if (actions != null) {
+                actions = PolicyUtils.expand(actions, system);
             }
-            if (pe.signers != null) {
-                pe.signers = PolicyUtils.expand(pe.signers, system);
+            if (signer != null) {
+                signer = PolicyUtils.expand(signer, system);
             }
         }
-        Certificate[] signers = (pe.signers == null) ? null : resolveSigners(
-                ks, pe.signers);
+        Certificate[] signers = (signer == null) ? null : resolveSigners(
+                ks, signer);
         try {
-            Class<?> klass = Class.forName(pe.klass);
+            Class<?> klass = Class.forName(className);
             if (PolicyUtils.matchSubset(signers, klass.getSigners())) {
-                return PolicyUtils.instantiatePermission(klass, pe.name,
-                        pe.actions);
+                return PolicyUtils.instantiatePermission(klass, name, actions);
             }
         }
         catch (ClassNotFoundException cnfe) {}
         //maybe properly signed class will be loaded later
-        return new UnresolvedPermission(pe.klass, pe.name, pe.actions, signers);
+        return new UnresolvedPermission(className, name, actions, signers);
     }
 
     /** 
@@ -348,19 +354,18 @@ public class DefaultPolicyParser impleme
     class PermissionExpander implements PolicyUtils.GeneralExpansionHandler {
 
         // Store KeyStore
-        private KeyStore ks;
+        private final KeyStore ks;
 
         // Store GrantEntry
-        private DefaultPolicyScanner.GrantEntry ge;
+        private final DefaultPolicyScanner.GrantEntry ge;
 
         /** 
          * Combined setter of all required fields. 
          */
-        public PermissionExpander configure(DefaultPolicyScanner.GrantEntry ge,
+        public PermissionExpander(DefaultPolicyScanner.GrantEntry ge,
                 KeyStore ks) {
             this.ge = ge;
             this.ks = ks;
-            return this;
         }
 
         /**
@@ -391,19 +396,18 @@ public class DefaultPolicyParser impleme
                             .hasNext();) {
                         DefaultPolicyScanner.PrincipalEntry pr = iter
                                 .next();
-                        if (pr.klass == null) {
+                        if (pr.getKlass() == null) {
                             // aliased X500Principal
                             try {
-                                sb.append(pc2str(getPrincipalByAlias(ks,
-                                        pr.name)));
+                                sb.append(pc2str(getPrincipalByAlias(ks, pr.getName())));
                             }
                             catch (Exception e) {
                                 if ( e instanceof SecurityException ) throw (SecurityException) e;
                                 throw new PolicyUtils.ExpansionFailedException(
-                                        Messages.getString("security.143", pr.name), e); //$NON-NLS-1$
+                                        Messages.getString("security.143", pr.getName()), e); //$NON-NLS-1$
                             }
                         } else {
-                            sb.append(pr.klass).append(" \"").append(pr.name) //$NON-NLS-1$
+                            sb.append(pr.getKlass()).append(" \"").append(pr.getName()) //$NON-NLS-1$
                                     .append("\" "); //$NON-NLS-1$
                         }
                     }
@@ -509,22 +513,21 @@ public class DefaultPolicyParser impleme
      */
     protected KeyStore initKeyStore(List<KeystoreEntry>keystores,
             URL base, Properties system, boolean resolve) {
-
         for (int i = 0; i < keystores.size(); i++) {
             try {
-                DefaultPolicyScanner.KeystoreEntry ke = keystores
-                        .get(i);
+                DefaultPolicyScanner.KeystoreEntry ke = keystores.get(i);
+                String url = ke.getUrl(), type = ke.getType();
                 if (resolve) {
-                    ke.url = PolicyUtils.expandURL(ke.url, system);
-                    if (ke.type != null) {
-                        ke.type = PolicyUtils.expand(ke.type, system);
+                    url = PolicyUtils.expandURL(url, system);
+                    if (type != null) {
+                        type = PolicyUtils.expand(type, system);
                     }
                 }
-                if (ke.type == null || ke.type.length() == 0) {
-                    ke.type = KeyStore.getDefaultType();
+                if (type == null || type.length() == 0) {
+                    type = KeyStore.getDefaultType();
                 }
-                KeyStore ks = KeyStore.getInstance(ke.type);
-                URL location = new URL(base, ke.url);
+                KeyStore ks = KeyStore.getInstance(type);
+                URL location = new URL(base, url);
                 InputStream is = AccessController
                         .doPrivileged(new PolicyUtils.URLLoader(location));
                 try {