Okay - Alan has pointed out that fairly recently (6888546)
setJavaLangAccess and booted() were in the opposite order to today.
Based on that I now think it is extremely low risk to change the order
as suggested.
Thanks,
David
On 29/02/2012 11:15 AM, David Holmes wrote:
Hi Mike,
On 29/02/2012 5:23 AM, Mike Duigou wrote:
On Feb 27 2012, at 20:50 , David Holmes wrote:
Hi Mike,
The problem with changing initialization order is that you can never
enumerate all the possible initialization paths. I don't see anything
problematic with the move in relation to Thread.currentThread() or
ThreadGroup.add(). But the call to setJavaLangAccess requires
initialization of SharedSecrets which in turn requires initialization
of Unsafe which in turn initializes sun.reflect.Reflection which in
turn initializes a bunch of Collection classes - and suddenly we may
have a whole swag of classes now being initialized before the VM
appears to be booted.
The class initialization I'm interested in does occur during the
SharedSecrets initialization path. Because, in the current
implementation, booted() hasn't been called the only way I can
determine if the class is being used before boot() is to check if the
SharedSecrets have been initialized. This ends up being cumbersome and
permanently leaves some checking logic in the execution path for what
is a very narrow window. I've looked at where isBooted is currently
being used and none of the uses would seem to be sensitive to being
called
Could Win32ErrorMode.initialize() now be called too late in some case?
What about sun.nio.cs.ext.ExtendedCharsets.init()?
...
If you then throw into the mix the possibility of different system
properties affecting initialization actions, or the existence of an
agent, then who knows what might actually happen.
Do you believe that the change is as a result too risky to consider?
Are there validations that could/should be done to reduce or eliminate
the risk?
I think this change may well have unanticipated consequences that might
take a long time to surface. If I was confident that regular testing
might expose these then I'd say go ahead, but I think these will be
obscure changes in very specific contexts that probably are not tested.
Given it is only a convenience fix is it worth any risk? You could
always add a new field to reflect the end of initializeSystemClass. Or
make booted an integer:
private static volatile int booted = 0;
public static void booted() {
booted++;
}
public static boolean isBooted() {
return booted > 0;
}
public static void fullyBooted() {
booted++;
}
public static boolean isFullyBooted() {
return booted > 1;
}
Cheers,
David
Cheers,
David
-----
On 28/02/2012 1:57 PM, Mike Duigou wrote:
Hello all;
This issue is a patch for review that I have split out of a larger
issue I'll be submitting later.
WEBREV @ http://cr.openjdk.java.net/~mduigou/7149320/0/webrev/
sun.misc.VM.booted() is currently called before setJavaLangAccess().
For code which uses the JavaLangAccess shared secrets it's
convenient to be able to check whether the secrets are initialized
without having to call sun.misc.SharedSecrets.getJavaLangAccess()
every time the secrets are used and checking for null. In particular
with the static inner class holder idiom it would be desirable to do :
static class Holder {
final sun.misc.JavaLangAccess ACCESS =
sun.misc.SharedSecrets.getJavaLangAccess();
}
...
if(sun.misc.VM.isBooted()&& Holder.ACCESS...
In my research on this issue I was unable to determine why
sun.misc.VM.booted() isn't the currently the last activity in
System.initSystemClass(). Neither of the two tasks which currently
follow it depend upon booted state in any way that I could
determine. I am tempted, thinking about it, to add a comment to
System.initSystemClass before the call to sun.misc.VM.booted()
asking future maintainers to leave boot() as the last activity or
explain why not.
I have done JTReg runs on linux x86, linux x64 and Solaris 64 which
incorporated this change without any problems encountered. It's
rather difficult to write a unit test for this issue as it requires
a class that is initialized after java.lang.System but before
java.lang.System.initSystemClass() and uses JavaLangAccess.
Thanks,
Mike
