----- "Martin Buchholz" <marti...@google.com> wrote:
> The JDK should find some other anti-inlining technique. > I would go one step further, and say that perhaps this anti-inlining is no longer necessary. I tried taking these methods out entirely and simply setting the in/out/err streams to null initially. This works. Probably due to some combination of 1) they are later on set from JNI code, and 2) JSR133, which among other things changed some rules about static final variables, notably: "Static final fields may only be modified in the class initializer that defines them, with the ex- ception of the java.lang.System.in, java.lang.System.out, and java.lang.System.err static fields, which can be modified by the java.lang.System.setIn, java.lang.System.setOut, and java.lang.System.setErr methods." I've attached a patch that (for me) has not caused any problems. Does anyone else have thoughts about this? cheers, jon > On Mon, Jun 28, 2010 at 07:14, <jon.vanal...@redhat.com> wrote: > > Hello all, > > > > We've been having a discussion on the downstream IcedTea bugzilla > about a potential jdk bug, and it seems prudent to bring it up here. > Link: > > > > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=394 > > > > Please ignore discussion there RE 32-bit *nix time overflow in 2038; > this is a glibc issue that Java cannot resolve. What I am more > concerned about is the complete incompatibility with any negative > currentTimeMillis() return value. > > > > TL/DR: System class does a (currentTimeMillis() > 0) check as part > of method that only exists to avoid inlining of the initial null > assignment of the in/out/err streams. So, if system time is before > January 1, 1970, java will not start. The bug reporter has given > several potential use cases where this could occur (summary in comment > 14 of bug report). > > > > In my opinion, this is a bug. The comment preceding the methods in > which this check occurs indicate that it is only to prevent inlining; > Java should not, IMO, care whether the system clock is set to 2367CE, > 2010, or 42BCE. Provided, of course, that the date falls within the > 64 bit signed long value that the currentTimeMillis() method returns. > In other words, I think that Java should not be concerned with > whether the system clock is in sync with real world time. > > > > I've tried changing the check to (currentTimeMillis() >= > Long.MIN_VALUE), to maintain the prevention of inlining while allowing > startup to proceed. Patch attached. This seems to work, in that when > system clock is before 1970 a program can actually start up. There > does not seem to be unwanted side effects when running a few simple > programs, although I have not done any real regression testing. > > > > Is this something that others think should be fixed in the JDK? Or > are Java users ultimately required to ensure that their system clock > is set accurately (and they are not time travelling hehe)? > > > > Related: I've been looking through other use of currentTimeMillis() > throughout the JDK, and I've found a few other places where there seem > to be assumptions made about the approximate expected return value. > If others are of the same opinion that Java should be agnostic about > what a "sensible" system time should be, then I'll summarize my > findings in a future post. > > > > Your thoughts are appreciated. > > > > cheers, > > > > jon
diff -r dc14ee238fe3 src/share/classes/java/lang/System.java --- a/src/share/classes/java/lang/System.java Wed Jun 02 12:53:35 2010 +0400 +++ b/src/share/classes/java/lang/System.java Wed Jul 07 10:49:07 2010 -0400 @@ -69,7 +69,7 @@ * corresponds to keyboard input or another input source specified by * the host environment or user. */ - public final static InputStream in = nullInputStream(); + public final static InputStream in = null; /** * The "standard" output stream. This stream is already @@ -96,7 +96,7 @@ * @see java.io.PrintStream#println(java.lang.Object) * @see java.io.PrintStream#println(java.lang.String) */ - public final static PrintStream out = nullPrintStream(); + public final static PrintStream out = null; /** * The "standard" error output stream. This stream is already @@ -110,7 +110,7 @@ * variable <code>out</code>, has been redirected to a file or other * destination that is typically not continuously monitored. */ - public final static PrintStream err = nullPrintStream(); + public final static PrintStream err = null; /* The security manager for the system. */ @@ -1093,26 +1093,6 @@ public static native String mapLibraryName(String libname); /** - * The following two methods exist because in, out, and err must be - * initialized to null. The compiler, however, cannot be permitted to - * inline access to them, since they are later set to more sensible values - * by initializeSystemClass(). - */ - private static InputStream nullInputStream() throws NullPointerException { - if (currentTimeMillis() > 0) { - return null; - } - throw new NullPointerException(); - } - - private static PrintStream nullPrintStream() throws NullPointerException { - if (currentTimeMillis() > 0) { - return null; - } - throw new NullPointerException(); - } - - /** * Initialize the system class. Called after thread initialization. */ private static void initializeSystemClass() {