[concurrency-interest] Here's why Atomic*FieldReference access checking is broken

Remi Forax forax at univ-mlv.fr
Sat Oct 4 05:39:43 EDT 2014


On 10/03/2014 05:13 PM, David M. Lloyd wrote:
> This discussion fizzled out last time it happened (2011 or 
> something?).  So to avoid that happening again, I'm going to 
> (hopefully clearly and in simple unambiguous terms) explain what the 
> problem is, why it should be fixed, why the problem occurs, and 
> exactly what should be done to fix the problem.
>
> What the problem is
> -------------------
> The expectation that users have when creating atomic field updaters is 
> that they have full access to any field that plain field accesses 
> would have (I believe this was reasonably well-established - or at 
> least implied - by all sides in the prior email thread).
>
> The problem, as observed by users, is that when you have code which 
> looks like this:
>
>    public class Foo {
>       private volatile int foo;
>       private static final AtomicIntegerFieldUpdater up =
>             AtomicIntegerFieldUpdater.newUpdater(Foo.class, "foo");
>       ...
>    }
>
> ...if you run with a security manager, and Foo.class.getClassLoader() 
> != AtomicIntegerFieldUpdater.class.getClassLoader(), you get an access 
> control exception unless Foo.class's ProtectionDomain includes the 
> "accessDeclaredMembers".  This is very contrary to the primary 
> expectation of users.
>
> Enter the obvious workaround - first grant the requisite permission, 
> then do this:
>
>    public class Foo {
>       private volatile int foo;
>       private static final AtomicIntegerFieldUpdater up =
>             AccessController.doPrivileged(
>                new PrivilegedAction<AtomicIntegerFieldUpdater>() {
>                   public AtomicIntegerFieldUpdater run() {
>                      return 
> AtomicIntegerFieldUpdater.newUpdater(Foo.class, "foo");
>                   }
>                }
>             );
>       ...
>    }
>
> Problem solved, yes?  No.  The newUpdater() method uses 
> getCallerClass() to get the class which is requesting access to the 
> field.  Since the caller class is actually Foo$1.class or some such, 
> the equality check in the constructor of the implementation fails, 
> causing an extra dynamic check to be run on every invocation.
>
> Here's the better, but still awful, non-obvious workaround:
>
>    public class Foo {
>       private volatile int foo;
>       private static AtomicIntegerFieldUpdater getIt() {
>          return AtomicIntegerFieldUpdater.newUpdater(Foo.class, "foo");
>       }
>
>       private static final AtomicIntegerFieldUpdater up =
>             AccessController.doPrivileged(
>                new PrivilegedAction<AtomicIntegerFieldUpdater>() {
>                   public AtomicIntegerFieldUpdater run() {
>                      return getIt();
>                   }
>                }
>             );
>       ...
>    }
>
> This solves the getCallerClass() problem, however it still requires 
> that Foo get the dangerous accessDeclaredMembers permission.

There is a better workaround, use a lambda, a lambda body is de-sugared 
to a private static method.

public class Foo {
       private volatile int foo;
       private static final AtomicIntegerFieldUpdater up =
             AccessController.doPrivileged(() -> 
AtomicIntegerFieldUpdater.newUpdater(Foo.class, "foo"));
       ...
}

It's just a workaround, as you said you still need to add 
accessDeclaredMembers permission.

cheers,
Rémi



More information about the Concurrency-interest mailing list