Thread safety and PermissionCollection

I just made an interesting discovery while implementing the PolicyConfiguration#addToUncheckedPolicy(PermissionCollection) method.

Notice anything unusual about the contract??? No?

Consider this: PermissionCollection subclasses need to be thread-safe, as the Javadoc says:

Subclass implementations of PermissionCollection should assume that they may be called simultaneously from multiple threads, and therefore should be synchronized properly.

OK, so when implementing this method, you might take the na??ve approach, as I did initially.?? The incoming PermissionCollection is thread-safe, so there's no need to synchronize on anything.?? Hopefully you're cringing at my haste:

private final PermissionCollection uncheckedPermissions = new Permissions();

@Override
public void addToUncheckedPolicy(final PermissionCollection permissionCollection) {
?? if (permissionCollection != null && this.uncheckedPermissions != null) {
?????? // Look, ma, no synchronization!
?????? final Enumeration<Permission> elements = permissionCollection.elements();
?????? if (elements != null) {
?? ?? ?? while (elements.hasMoreElements()) {
?? ?? ?? ?? final Permission permission = elements.nextElement();
?????? ?? ?? if (permission != null && !this.uncheckedPermissions.implies(permission)) {
?????? ?? ?? ?? this.uncheckedPermissions.add(permission);
?????? ?? ?? }
?????????? }
?????? }
?? }
}

You could probably get away with this.?? After all, you know that the uncheckedPermissions member variable is guaranteed to be thread-safe (java.security.Permissions instances, like all PermissionCollection subclasses, must be "synchronized properly").?? Surely there's nothing more to worry about?

Wrong.?? JACC makes no guarantees one way or the other about what is happening to the incoming PermissionCollection that you're adding in this method.?? This PermissionCollection could be being modified by some other thread while you're processing it.?? So your elements() call–your enumeration of the individual Permission instances "inside" that PermissionCollection–will be inconsistent and broken.

OK, you think (I think), I'll just synchronize on the incoming PermissionCollection object.?? But there's nothing in the PermissionCollection documentation that indicates that PermissionCollection objects must synchronize on themselves during modification operations (such as add()).?? They can synchronize on whatever they want and are under no obligation to tell you what that mutex is going to be.?? Nine times out of then you're going to be handed a Permissions instance, of course, and if you go look at the source, yes, indeed, the PermissionCollection implementation inside that synchronizes on itself.?? But it certainly doesn't have to, and you shouldn't rely on it.

In the absence of further guarantees, all you can do is add the incoming PermissionCollection to a set of such PermissionCollections, and then, when you actually need to check permissions, walk through the set one by one and call implies() on each PermissionCollection.

The takeaway here is that in any code that you're using that enumerates a PermissionCollection, you're probably doing it wrong unless you have control of "both sides" of the PermissionCollection–unless you control both when Permissions are added to it and when they are enumerated.