System.currentTimeMillis check in System class during startup.

[Martin Buchholz]

Hi Jon, Alan, Chris,

Jon, I have a more conservative version of your patch,
that uses blank finals, only calls currentTimeMillis once instead of 3 times,
and only fails in the unlikely event of currentTimeMillis == Long.MIN_VALUE.

I'm too afraid to remove the anti-inlining entirely.

http://cr.openjdk.java.net/~martin/webrevs/openjdk7/TimeTravel/

Could someone at Oracle file a bug and provide review?
Alan? Chris?

Martin

On Wed, Jul 7, 2010 at 08:00,  <jon.vanalten at redhat.com> wrote:
>
> ----- "Martin Buchholz" <martinrb at 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.vanalten at 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
>