You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@river.apache.org by Peter <ji...@zeus.net.au> on 2011/10/27 15:55:18 UTC
Java security Policy - concurrency issues
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.
Re: Java security Policy - concurrency issues
Posted by Peter <ji...@zeus.net.au>.
----- Original message -----
> 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...
Interesting, read it, mutate it, write it back, didn't think of that.
You're right it's not pretty.
>
> Should we create an issue in the JDK bugzilla?
Yes. Actually, I don't think they should use any synchronisation, this can be provided by a wrapper class similar to collections.
Cheers,
Peter.
>
> 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.
> > > >
> > >
> >
> >
>
Re: Java security Policy - concurrency issues
Posted by Gregg Wonderly <ge...@cox.net>.
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.
>>>
>>
>
>
Re: Java security Policy - concurrency issues
Posted by Peter <ji...@zeus.net.au>.
That's exactly what the original implementers needed to do, make those fields volatile.
They're private implementation fields though.
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.
> >
>
Re: Java security Policy - concurrency issues
Posted by Gregg Wonderly <gr...@wonderly.org>.
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.
>