RFR 8098798: Thread.join(ms) on Linux still affected by changes to the time-of-day clock

[Roger Riggs]

Hi Martin, Ivan,

Thanks for the suggestions.

Update in place:
http://cr.openjdk.java.net/~rriggs/webrev-thread-early-8098798/

On 8/29/2018 5:36 PM, Martin Buchholz wrote:
> Thanks for taking this on.
> Wait loops are notoriously hard to get right...
>
> One sharp corner is that wait(0) waits forever, and TimeUnit 
> conversion truncates.  You can probably avoid those problems via 
> TimeUnit.timedWait.
>
Not trivial since a long cannot hold the combined time of millis(long) 
and nanos (long) in a TimeUnit(Nanos)
and the cumulative wait time needs to be measured by System.nanoTime().

> In code like this in j.u.c. we try to optimize away the call to 
> nanoTime when waiting is not necessary, by using a special 
> "uninitialized" initial value for remaining nanos, e.g. in 
> FutureTask.awaitDone
>
>         long startTime = 0L;    // Special value 0L means not yet parked
ok
>
> (I prefer the variable name "startTime")
ok
>
> (j.u.c. code can also be improved)
>
> (there's a pre-existing buglet - we should probably check for overflow 
> when millis = MAX_VALUE and nanos > 0 (sigh))
ok,
>
> (I would reorder clauses to first check the common case millis > 0, 
> then millis == 0, last the error case millis < 0)
ok
>
> (it's a bad API that millis < 0 is an error)
too late for a behavior change though I suppose its in the direction of 
not getting error instead of the opposite....


An Observation:

Join(ms) and join(ms, ns) might wait a bit longer than strictly 
necessary because the bottom out in Object.wait(ms).
It might be better if both ended up calling Object.wait(ms, ns).
But since Object.wait(ms,ns) rounds up to Object.wait(ms) that won't 
make a difference and to take advantage
of a finer clock resolution would mean native/vm changes to support 
object.wait(ms, ns).

I'm inclined to address only the immediate issues raised in the early 
return and the clock dependency now.

(BTW, I found no tests for Thread.sleep or .join.)

Thanks, Roger

>
>
> On Wed, Aug 29, 2018 at 1:06 PM, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto:Roger.Riggs at oracle.com>> wrote:
>
>     Please review changes for:
>
>     8098798: Thread.join(ms) on Linux still affected by changes to the
>     time-of-day clock
>          Use System.nanoTime to compute any remaining delay
>
>     8210004: Thread.sleep(millis, nanos) timeout returns early
>          Avoid an early return by rounding the milliseconds up if
>     there are any nanoseconds as was done in Object.wait(ms, ns).
>
>     (If its not appropriate to do the two reviews together, I can
>     split them).
>
>     Webrev:
>     http://cr.openjdk.java.net/~rriggs/webrev-thread-early-8098798/
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-thread-early-8098798/>
>
>     Thanks, Roger
>
>