RFR: 8172048: Re-examine use of AtomicReference in java.security.Policy

[Claes Redestad]

<dropped security-libs>

Hi Peter,

agreed, it appears this assert could be replaced by a test - similar to 
what was already
done for VarHandle.AccessMode[1] - which might increase robustness of 
early use of
VarHandles and give our core reflection libraries the freedom to use 
VarHandles.

Paul?

/Claes

[1] https://bugs.openjdk.java.net/browse/JDK-8160439

On 12/28/2016 01:07 AM, Peter Levart wrote:
> Hi Claes,
>
> A nice find! This is certainly a straightforward and low-risk fix for 
> breaking the initialization cycle problem with JDK-8062389.
>
> Related to VarHandles, the call trace of the initialization cycle also 
> includes the following part of code in VarHandle:
>
>
>         AccessMode(final String methodName, AccessType at) {
>             this.methodName = methodName;
>             this.at = at;
>
>             // Assert that return type is correct
>             // Otherwise, when disabled avoid using reflection
>             assert at.returnType == getReturnType(methodName);
>         }
>
> ...
>
>         private static Class<?> getReturnType(String name) {
>             try {
>                 Method m = VarHandle.class.getMethod(name, 
> Object[].class);
>                 return m.getReturnType();
>             }
>             catch (Exception e) {
>                 throw newInternalError(e);
>             }
>         }
>
>
> ... where enabling assertions (tests enable assertions) causes the 
> initialization of VarHandle(s) to involve reflection. This is also a 
> place that could be made more robust, perhaps by devising a special 
> test that verifies method return types, instead of embedding the check 
> into the AccessMode constructor...
>
> Regards, Peter
>
>
> On 12/27/2016 03:04 PM, Claes Redestad wrote:
>> Hi,
>>
>> since java.util.concurrent.AtomicReference was changed to use a
>> VarHandle internally, using it from within the security libraries can
>> lead to hard to diagnose bootstrap cycles (since VarHandles has to do
>> doPrivileged calls during setup). The need to initialize VarHandles is
>> also cause for a small startup regression for any application run with
>> a security manager.
>>
>> The use of AtomicReference in java.security.Policy is not really
>> motivated, though, since only the .get/.set methods are used, thus a
>> rather straight-forward fix is to convert the code to use a volatile
>> reference instead with identical semantics:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8172048
>> Webrev: http://cr.openjdk.java.net/~redestad/8172048/webrev.01/
>>
>> While a rather insignificant startup improvement in and off itself,
>> this would help to unblock the attempted fix to resolve
>> https://bugs.openjdk.java.net/browse/JDK-8062389
>>
>> Thanks!
>>
>> /Claes
>