Request for reviews (S): 6863420: os::javaTimeNanos() go backward on Solaris x86

[David Holmes - Sun Microsystems]

Hi Vladimir,

This is a good catch in some quite old code! I wonder if there are any 
other word-tearing bugs lurking ... "normal" CAS-loops wouldn't be 
affected because the word-tearing would cause the CAS to fail, but any 
"optimized" ones where the value normally updated by CAS is simply read 
and used, would suffer the same problem as here.

But it seems odd to define a platform-independent Atomic::load(jlong, 
jlong) function but then only implement it for 32-bit Solaris x86 and 
use conditional compilation at the call site. It would seem much cleaner 
to always use Atomic::load(jlong, jlong) and have it evaporate to 
nothing on platforms for which it is a direct load. Given the spread of 
the JDK I suspect there are other platforms where non-atomic 64-bit 
loads could be an issue.

Cheers,
David Holmes

Vladimir Kozlov said the following on 07/24/09 10:56:
> http://cr.openjdk.java.net/~kvn/6863420/webrev.00
> 
> Fixed 6863420: os::javaTimeNanos() go backward on Solaris x86
> 
> Problem:
> The problem is non atomic load from max_hrtime in getTimeNanos()
> on platforms where long is kept in 2 32-bit register.
> The loaded value could be invalid (> current time) since registers
> are loaded separately and store could happened in between.
> It could be returned as result if it is greater then current time.
> And if the next call getTimeNanos() returns the correct time
> it will be less then previous result.
> 
> Solution:
> Use new atomic long load method Atomic::load() which uses
> FPU to move long value atomically.
> 
> Added the regression test which shows the problem.
> 
> Reviewed by:
> 
> Fix verified (y/n): y, regression test
> 
> Other testing:
> JPRT
>