On Thu, 14 Mar 2024 23:23:07 GMT, Brent Christian <bchri...@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."_
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   further tweaks to reachability

src/java.base/share/classes/java/lang/ref/Reference.java line 402:

> 400:      * method is called, the garbage collector may already be in the 
> process of
> 401:      * (or already completed) clearing and/or enqueueing this reference.
> 402:      *

Either this is an extra blank line, or you need a `<p>` here.

src/java.base/share/classes/java/lang/ref/Reference.java line 496:

> 494:      * Actions in a thread prior to calling
> 495:      * {@code enqueue} <b><i>successfully</i></b>
> 496:      * <a 
> href="{@docRoot}/java.base/java/util/concurrent/package-summary.html#MemoryVisibility"><i>happen-before</i></a>

Editorial. The text here says

> Actions in a thread prior to calling `enqueue` successfully _happen-before_ 
> the reference is removed...

This could be confusing, because "successfully" might be read to modify 
"happen-before". This raises questions such as "Is it possible for something to 
happen-before unsuccessfully?" Of course you want "successfully" to modify 
"enqueue" because you're relying on the definition of "successful" given 
previously. Suggest rewording:

> Actions in a thread prior to successful calls to `enqueue` _happen-before_ 
> the reference is removed...

src/java.base/share/classes/java/lang/ref/package-info.java line 127:

> 125:  * thread prior to a {@link Reference#reachabilityFence 
> Reference.reachabilityFence(x)}
> 126:  * <i>happen-before</i> cleanup code for {@code x} runs on the cleaner 
> thread.
> 127:  *

Extra blank line or need `<p>`.

src/java.base/share/classes/java/lang/ref/package-info.java line 146:

> 144:  *
> 145:  * <li> An object is <em>strongly reachable</em> if it can be accessed 
> without
> 146:  * traversing the referent of a Reference object.

Here, we want the definition of "strongly reachable" to build on the JLS 
definition of "reachable" quoted above. Suggest something like

> An object is **strongly reachable** if it is reachable and if it can be 
> accessed without traversing the referent of a Reference object.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1527074234
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1527080061
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1527074924
PR Review Comment: https://git.openjdk.org/jdk/pull/16644#discussion_r1527076814

Reply via email to