RFR: 8314480: Memory ordering spec updates in java.lang.ref

[Viktor Klang]

On Wed, 15 Nov 2023 22:33:03 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Classes in the `java.lang.ref` package would benefit from an update to bring the spec in line with how the VM already behaves. The changes would focus on _happens-before_ edges at some key points during reference processing.
>> 
>> A couple key things we want to be able to say are:
>> - `Reference.reachabilityFence(x)` _happens-before_ reference processing occurs for 'x'.
>> - `Cleaner.register()` _happens-before_ the Cleaner thread runs the registered cleaning action.
>> 
>> This will bring Cleaner in line (or close) with the memory visibility guarantees made for finalizers in [JLS 17.4.5](https://docs.oracle.com/javase/specs/jls/se18/html/jls-17.html#jls-17.4.5):
>> _"There is a happens-before edge from the end of a constructor of an object to the start of a finalizer (§12.6) for that object."_
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 495:
> 
>> 493:      * <a href="{@docRoot}/java.base/java/util/concurrent/package-summary.html#MemoryVisibility"><i>happen-before</i></a>
>> 494:      * the reference is removed from the queue by {@link ReferenceQueue#poll}
>> 495:      * or {@link ReferenceQueue#remove}.
> 
> Should there be an explicit statement about an unsuccessful call having no memory consistency effects?

Perhaps something along the lines of "A failed {@code enqueue} has no defined memory consistency effects."?

> src/java.base/share/classes/java/lang/ref/Reference.java line 505:
> 
>> 503:      * to return this reference even though the referent is still strongly
>> 504:      * reachable. That is, before the referent has reached the expected
>> 505:      * reachability level.
> 
> The wording here is kind of awkward. We could try to wordsmith it, but there is also the possibility that there are valid use cases for using enqueue(), which makes this caution seem misplaced. An alternative would simply be to omit this paragraph.

Might be better to leave out the last sencence?

> src/java.base/share/classes/java/lang/ref/Reference.java line 564:
> 
>> 562:      * {@code reachabilityFence(x)}
>> 563:      * <a href="{@docRoot}/java.base/java/util/concurrent/package-summary.html#MemoryVisibility"><i>happen-before</i></a>
>> 564:      * the garbage collector clears any reference to {code x}.
> 
> This is a fairly low-level specification, so the relationship described here is "reachabilityFence **hb** clearing". The relationships "clearing **hb** enqueue" and "enqueue **hb** dequeue" are described elsewhere. Thus the mention of Cleaner above seems misplaced.
> 
> However, the whole chain of relationships
> 
>> RF **hb** clearing **hb** enqueue **hb** dequeue **hb** cleaning-action
> 
> is important. Should this be described somewhere?

Perhaps in each of these places add a link to #Memory Consistency Properties

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400247244
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400251257
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1400254380