river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <j...@zeus.net.au>
Subject Re: com.sun.jini.thread lock contention
Date Mon, 07 Jun 2010 19:00:33 GMT
Hi Gregg,

I see it now, it's the SoftCache that is accessed just before the 
security check.

Yep this could use some refactoring.  I'll play with it on the weekend, 
have you experimented with it yet?

Cheers,

Peter.

    /** cache of subclass security audit results */
    private static final SoftCache subclassAudits = new SoftCache(10);

   /**
     * Verifies that this (possibly subclass) instance can be constructed
     * without violating security constraints: the subclass must not 
override
     * security-sensitive non-final methods, or else the
     * "enableContextClassLoaderOverride" RuntimePermission is checked.
     */
    private static boolean isCCLOverridden(Class cl) {
    if (cl == Thread.class)
        return false;
    Boolean result = null;
    synchronized (subclassAudits) {


    /*
     * Do we have the required permissions?
     */
    if (security != null) {
        if (isCCLOverridden(getClass())) {
            security.checkPermission(SUBCLASS_IMPLEMENTATION_PERMISSION);
        }
    }

<---------------Thread.java Selected source code 
below--------------------------->

 public Thread(String name) {
        init(null, null, name, 0);
    }

    public final void checkAccess() {
        SecurityManager security = System.getSecurityManager();
        if (security != null) {
            security.checkAccess(this);
        }
    }

    private void init(ThreadGroup g, Runnable target, String name,
                      long stackSize) {
    Thread parent = currentThread();
    SecurityManager security = System.getSecurityManager();
    if (g == null) {
        /* Determine if it's an applet or not */
       
        /* If there is a security manager, ask the security manager
           what to do. */
        if (security != null) {
        g = security.getThreadGroup();
        }

        /* If the security doesn't have a strong opinion of the matter
           use the parent thread group. */
        if (g == null) {
        g = parent.getThreadGroup();
        }
    }

    /* checkAccess regardless of whether or not threadgroup is
           explicitly passed in. */
    g.checkAccess();

    /*
     * Do we have the required permissions?
     */
    if (security != null) {
        if (isCCLOverridden(getClass())) {
            security.checkPermission(SUBCLASS_IMPLEMENTATION_PERMISSION);
        }
    }


        g.addUnstarted();

    this.group = g;
    this.daemon = parent.isDaemon();
    this.priority = parent.getPriority();
    this.name = name.toCharArray();
    if (security == null || isCCLOverridden(parent.getClass()))
        this.contextClassLoader = parent.getContextClassLoader();
    else
        this.contextClassLoader = parent.contextClassLoader;
    this.inheritedAccessControlContext = AccessController.getContext();
    this.target = target;
    setPriority(priority);
        if (parent.inheritableThreadLocals != null)
        this.inheritableThreadLocals =
        ThreadLocal.createInheritedMap(parent.inheritableThreadLocals);
        /* Stash the specified stack size in case the VM cares */
        this.stackSize = stackSize;

        /* Set thread ID */
        tid = nextThreadID();
    }

   /**
     * Verifies that this (possibly subclass) instance can be constructed
     * without violating security constraints: the subclass must not 
override
     * security-sensitive non-final methods, or else the
     * "enableContextClassLoaderOverride" RuntimePermission is checked.
     */
    private static boolean isCCLOverridden(Class cl) {
    if (cl == Thread.class)
        return false;
    Boolean result = null;
    synchronized (subclassAudits) {
        result = (Boolean) subclassAudits.get(cl);
        if (result == null) {
        /*
         * Note: only new Boolean instances (i.e., not Boolean.TRUE or
         * Boolean.FALSE) must be used as cache values, otherwise cache
         * entry will pin associated class.
         */
        result = new Boolean(auditSubclass(cl));
        subclassAudits.put(cl, result);
        }
    }
    return result.booleanValue();
    }


Gregg Wonderly wrote:
> I am on the road so not able to be most timely.  The issue is that 
> there is a class level lock applied when a thead instance is created 
> with a subclass of thread.  Look at the constructors got the details.
>
> Gregg Wonderly
>
> Sent from my iPhone
>
> On Jun 4, 2010, at 11:02 PM, Peter Firmstone <jini@zeus.net.au> wrote:
>
>> Hi Gregg,
>>
>> The contention appears to be around the use of the existing single 
>> thread synchronized Policy implementations like DynamicPolicyProvider.
>>
>> There are numerous places where Thread is checked (not just 
>> subclasses) by the SecurityManager using the checkAccess(Thread) 
>> method, this is delegated to AccessController.checkPermission(perm)
>>
>> checkPermission(perm) gets the AccessControlContext of the currently 
>> executing stack, AccessControlContext.optimize() is called to remove 
>> duplicate PermissionDomain's
>>
>> then AccessControlContext.checkPermission(perm) is called to check 
>> the permission's of all the ProtectionDomain's on the current stack, 
>> the ProtectionDomain's on the stack are iterated through, the first 
>> ProtectionDomain.implies(Permission) that returns false returns 
>> causes the AccessControlContext to return false.
>>
>> This means that if your thread is executing with several CodeSource's 
>> it will have to check several ProtectionDomains.
>>
>> Each ProtectionDomain will check the policy, if the ProtectionDomain 
>> was created with the dynamic policy domain constructor:
>>
>> public ProtectionDomain(CodeSource codesource,
>>               PermissionCollection permissions,
>>               ClassLoader classloader,
>>               Principal[] principals)
>>
>> The ProtectionDomain first check's the Policy for the permission, 
>> then it checks it's own private PermissionCollection permissions 
>> passed in via the constructor, or that which has been merged with the 
>> permissions from the Policy.
>> Policy.getPermissions(ProtectionDomain)
>>
>> In my RevokeablePolicy implementation, getPermissions doesn't return 
>> any dynamic Permission grant's, if it did, they could never be 
>> revoked as they would become merged with the ProtectionDomains 
>> private PermissionCollection.
>>
>> So to sum up, you should get much better mileage if you try out my 
>> new ConcurrentDynamicPolicy implementation.
>>
>> I know you've got a privately forked copy of River, but if you simply 
>> copy concurrent_policy_utils.jar and jsk-policy.jar from River's 
>> Hudson build, to your jre/lib/ext directory, you should be able to 
>> utilise it.
>>
>> The ConcurrentDynanicPolicyProvider has passed all qa tests and jtreg 
>> tests, I'm just waiting for someone like yourself to report back 
>> concurrency improvements.
>>
>> Cheers,
>>
>> Peter.
>>
>>
>> Gregg Wonderly wrote:
>>> The TaskManager class has a subclass of Thread that it uses to run 
>>> tasks.  The problem with subclassing Thread, is that there is a lock 
>>> that is used in Thread to see if some methods are overridden so that 
>>> security can be maintained.  So, in an environment where Thread is 
>>> used with a SecurityManager, it is better to use a Runnable to 
>>> encapsulate the custom code, instead of a Thread.
>>>
>>> The TaskThread class should be a Runnable instead of a Thread, and 
>>> then there are just 3 other places where it is referenced that need 
>>> some adjustment.  The thread that is running inside of the runnable 
>>> needs to be put into a per instance variable for one place where 
>>> interrupt processing is handled.
>>>
>>> Gregg Wonderly
>>>
>>
>


Mime
View raw message