RFR: JDK-8035663 Suspicious failure of test java/util/concurrent/Phaser/FickleRegister.java

[David Holmes]

Still looking for Reviewer please.

Thanks,
David

On 19/11/2014 3:17 PM, David Holmes wrote:
> webrev:
>
> http://cr.openjdk.java.net/~dholmes/8035663/webrev.jdk9/
>
> This test failure exposed a number of issues with the logic in
> unsafe.cpp for handling atomic updates of Java long fields on platforms
> without any direct support for a 64-bit CAS operation - platforms for
> which supports_cx8 is not true. This only impacts our SE Embedded PPC32
> platform (where we have been using this fix for some time now) but in
> case other such platforms came along I wanted to get this pushed to
> mainline.
>
> What the unsafe code did was to use the object containing the field as a
> lock object for reading and writing the field. This seems reasonable on
> the surface but in fact had a fatal flaw - because we were locking a
> Java-level visible object inside what was considered to be a lock-free
> code-path by the application and library logic, we could actually induce
> a deadlock - which is why the test failed.
>
> In addition the code had two further flaws:
>
> 1. Because the field could also be updated via direct assignment in Java
> code the unsafe code needed to perform an Atomic::load of the field. And
> for good measure we also employ an Atomic::store to ensure no
> interference with direct reads of the field in Java code.
>
> 2. The address of the field was being calculated before using the
> ObjectLocker to lock the object, but locking could encounter a safepoint
> check allowing the object to relocated by the GC, and we would then use
> a stale address.
>
> To fix all of this we:
> - introduce a special Mutex to use instead of the deadlock-inducing Java
> object
> - use Atomic::load and Atomic::store to access the jlong field
> - avoid safepoints when locking (alternatively you could ensure you
> calculate the address after acquiring the lock )
>
> Thanks,
> David