Mailing-List: contact dev-help@river.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@river.apache.org
Received-SPF: pass (athena.apache.org: domain of gergg@cox.net designates
 68.230.241.218 as permitted sender)
Message-ID: <4EA9B667.2050309@cox.net>
Date: Thu, 27 Oct 2011 14:52:07 -0500
From: Gregg Wonderly <gergg@cox.net>
User-Agent: Mozilla/5.0 (Windows NT 6.0;
 rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1
MIME-Version: 1.0
To: dev@river.apache.org
CC: Peter <jini@zeus.net.au>
Subject: Re: Java security Policy - concurrency issues
References: <1319723718.2232.4.camel@Nokia-N900-51-1>
   <4EA97EE9.6010507@wonderly.org> <1319739475.2380.6.camel@Nokia-N900-51-1>
In-Reply-To: <1319739475.2380.6.camel@Nokia-N900-51-1>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 10/27/2011 1:17 PM, Peter wrote:
> That's exactly what the original implementers needed to do, make those fields volatile.
>
> They're private implementation fields though.

Okay, but are there any usage patterns where we could add the use of a volatile? 
  The JMM says that multiple non-volatile fields can be made visible by a single 
volatile write and then another thread making a volatile read.  So if we are 
adding properties, we should do a volatile write after that, and then if there 
is a place before the use of permissioncollection, by another thread, that we 
can force a volatile read of the same volatile field, then that should fix the 
visibility.  It's not pretty...

Should we create an issue in the JDK bugzilla?

Gregg

>
> Trouble is, none of these old jvm homogenous PermissionCollection's have been exposed to any more than single threads before and the last thing I want to do is reimplement them.  They're supposed to be thread safe but many have visibility issues.
>
> Considering java security policy is a occassional write, multi read, it should be simple to make it scale very well, using immutability and concurrency utils.  There's just some legacy cruft that spoils it a little.
>
> I guess I could make a wrapper class that uses volatile and write replace,  but then if it changes you still have to replace the underlying PermissionCollection, and still wear the synchronisation cost.
>
> Cheers,
>
> Peter.
>
> Cheers,
>
> Peter.
>
> ----- Original message -----
>> What about a volatile as the visibility control?  Write after update, read
>> before access?  It would at least expose the changes to other threads, not be a
>> lock, and represent a fairly limited overhead on most hardware.
>>
>> Gregg
>>
>> On 10/27/2011 8:55 AM, Peter wrote:
>>> The problem:
>>>
>>> Stale references allowed and noted in comments:
>>>
>>> java.security.Permissions
>>> java.security.BasicPermissions.BasicPermissionCollection
>>>
>>> The stale reference in Permissions is an AllPermission object - an
>>> optimisation.  If a thread doesn't see the current value, it just checks the
>>> internal Map, which is synchronised, no biggy.
>>>
>>> Problem is, Permissions is a heterogenous PermissionCollection, it contains a
>>> Map, synchronzed thread access, which prevents a similar optimisation in the
>>> homogenous BasicPermissionCollection from being seen in the stale state.
>>>
>>> Every ProtectionDomain has its own Permissions and each Permission class type
>>> has it's own unique PermissionCollection shared with all others with the same
>>> type for a ProtectionDomain.
>>>
>>> I replaced Permissions with a class called ConcurrentPermissions that uses a
>>> ConcurrentMap
>>>
>>> Trouble is BasicPermissionCollection is no longer protected by synchronization
>>> in Permissions.  BasicPermissionCollection now exposed to multiple threads has
>>> a stale reference optimisation for wildcard * permissions.
>>>
>>> What happens in my concurrent policy implementation is the Permission isn't
>>> necessarily found in the BasicPermissionCollection by a second thread, so it
>>> checks the PermissionGrants (immutable objects that contain data from policy
>>> files or dynamic grants) again and adds all the permissions to
>>> BasicPermissionCollection again.    So it doesn't fail, but it doesn't scale
>>> well with contention, because you've still got the synchronisation bottleneck,
>>> can't see the Permission and have to process again, wasting resources, on the
>>> second occassion.
>>>
>>> Problem is, BasicPermissionCollection is the bread and butter
>>> PermissionCollection implementation many Permission classes use.
>>>
>>> Now you have to remember, these classes were designed well before concurrency
>>> was ever a consideration.  Nowadays these classes would be immutable, since
>>> policy's don't change much, they're mostly read access.
>>>
>>> But I can't change it because many are part of the decision process.
>>>
>>> Now I could put a synchronized wrapper PermissionCollection class around these
>>> things, which fixes the bug, creating long lived objects that live on the heap
>>> and will likely cause L2 cache misses or contended locks.
>>>
>>> How about something different?
>>>
>>> Create the PermissionCollection's on demand, then discard immediately after
>>> use.  The Permission objects themselves are long lived immutable objects.
>>>
>>> Why?
>>>
>>> It'll be used only by one thread, so the jvm will optimise out the
>>> synchronised locks.
>>>
>>> The object will be created on the threads local memory stack, instead of the
>>> heap and die in the young generation, so it doesn't incur gc heap generation
>>> movements or memory heap copy to cpu cache stalls.
>>>
>>> But what about single thread applications or those with few threads and little
>>> contention?  They would run slower, although object allocation costs aren't as
>>> bad as people think, say 10 to 20 cpu cycles compared to 200 for a cache miss,
>>> or worse for a contended lock.
>>>
>>> Pattern matching of strings is the most expensive computation of most
>>> permission decisions and has to be repeated for every ProtectionDomain on the
>>> call stack for each thread, the impact on single core machines won't be much.
>>> I can test for that, but not the high end stuff.
>>>
>>> Arrghh decisions!  Not enough test hardware.
>>>
>>> Cheers,
>>>
>>> Peter.
>>>
>>
>
>