P3
FULL PRODUCT VERSION :
ADDITIONAL OS VERSION INFORMATION :
Experienced by a downstream developer with Oracle JDK1.6 on OSX.
A DESCRIPTION OF THE PROBLEM :
Our policy provider thread confines permission checks, such that PermissionCollection objects are created on demand, while Permission objects are shared among threads.
The Permission objects are stored in immutable objects named PermissionGrant, each of which mirrors a grant statement in a policy file.
A policy is made up of numerous permission grants. RFC3986 compliant normalisation is used on CodeSource URL (converted to URI) based on the CodeSource.implies javadoc to determine if a PermissionGrant applies to a particular ProtectionDomain. DNS name resolution is not performed on CodeSource URI. Policy files are parsed at startup.
Permission objects were stored in a TreeSet<Permission> () wrapped in a Collections.unmodifiableSet() referenced by a final field. A Comparator<Permission> is used for ordering. All fields are final in PermissionGrant and the this reference doesn't escape during construction.
When a SocketPermission is checked by the SecurityManager, it consults the policy provider, which constructs a thread confined Permissions object on demand, Permission objects are added to it in preferred order (to optimise implies calls) from each applicable PermissionGrant. Then Permissions.implies(Permission) is invoked by the calling thread. The Permissions PermissionCollection is discarded after the method completes.
SocketPermission returned false when it was obvious from security debug output that it was true.
The TreeSet was replaced with a ConcurrentSkipListSet and Permission.getAction() was called on all Permission objects, prior to being added to the ConcurrentSkipListSet, by the policy file parsing thread.
Although we were able to work arround the issue in our software, the Permission javadoc states that Permission objects are immutable like String, it also states however that creating an action string representation may be deferred.
Suggestion:
1. Fields in SocketPermission be changed to volatile or final (where applicable) to avoid data race conditions between threads.
2. That getAction when deferred, doesn't change the effective immutablity of Permission implementations, such that any such call shall be idempotent and the result stored in a volatile reference.
REPRODUCIBILITY :
This bug can be reproduced occasionally.
CUSTOMER SUBMITTED WORKAROUND :
Call Permission.getAction() after construction but before safe publication to other threads. Use a volatile reference or concurrent collection to safely publish the Permission.
For safety we used a concurrent collection, although previously we relied on final field safe publication, the reordering constraints for final fields appear more likely to produce data races with SocketPermission, although this last statement hasn't been verified.
ADDITIONAL OS VERSION INFORMATION :
Experienced by a downstream developer with Oracle JDK1.6 on OSX.
A DESCRIPTION OF THE PROBLEM :
Our policy provider thread confines permission checks, such that PermissionCollection objects are created on demand, while Permission objects are shared among threads.
The Permission objects are stored in immutable objects named PermissionGrant, each of which mirrors a grant statement in a policy file.
A policy is made up of numerous permission grants. RFC3986 compliant normalisation is used on CodeSource URL (converted to URI) based on the CodeSource.implies javadoc to determine if a PermissionGrant applies to a particular ProtectionDomain. DNS name resolution is not performed on CodeSource URI. Policy files are parsed at startup.
Permission objects were stored in a TreeSet<Permission> () wrapped in a Collections.unmodifiableSet() referenced by a final field. A Comparator<Permission> is used for ordering. All fields are final in PermissionGrant and the this reference doesn't escape during construction.
When a SocketPermission is checked by the SecurityManager, it consults the policy provider, which constructs a thread confined Permissions object on demand, Permission objects are added to it in preferred order (to optimise implies calls) from each applicable PermissionGrant. Then Permissions.implies(Permission) is invoked by the calling thread. The Permissions PermissionCollection is discarded after the method completes.
SocketPermission returned false when it was obvious from security debug output that it was true.
The TreeSet was replaced with a ConcurrentSkipListSet and Permission.getAction() was called on all Permission objects, prior to being added to the ConcurrentSkipListSet, by the policy file parsing thread.
Although we were able to work arround the issue in our software, the Permission javadoc states that Permission objects are immutable like String, it also states however that creating an action string representation may be deferred.
Suggestion:
1. Fields in SocketPermission be changed to volatile or final (where applicable) to avoid data race conditions between threads.
2. That getAction when deferred, doesn't change the effective immutablity of Permission implementations, such that any such call shall be idempotent and the result stored in a volatile reference.
REPRODUCIBILITY :
This bug can be reproduced occasionally.
CUSTOMER SUBMITTED WORKAROUND :
Call Permission.getAction() after construction but before safe publication to other threads. Use a volatile reference or concurrent collection to safely publish the Permission.
For safety we used a concurrent collection, although previously we relied on final field safe publication, the reordering constraints for final fields appear more likely to produce data races with SocketPermission, although this last statement hasn't been verified.