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