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

[Hans Boehm]

Is it worth keeping the discussion starting with "It is sometimes possible
to better encapsulate ..." and the associated example code? I find this
example extremely unconvincing. It's very hard to construct a case in which
you can safely use the result of getExternalResource(). And I don't want to
encourage writing getters for things like this; if you use them from a
static method, or somebody decides to make it public, things get really
messy really quickly.

This example kind of gives the impression that we have a solution to
reachabilityFence() proliferation. I don't think we do. There may be a
difference of opinion about whether it's worth fixing, but I don't think we
should deny the existence of the problem.

On Thu, Mar 21, 2024 at 7:11 PM David Holmes <dholmes at openjdk.org> wrote:

> On Thu, 21 Mar 2024 23:38:30 GMT, Brent Christian <bchristi 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."_
> >
> > Brent Christian has updated the pull request incrementally with one
> additional commit since the last revision:
> >
> >   Elaborate on 'surprising and undesirable effects' in
> reachabilityFence()
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 576:
>
> > 574:      * detects that there is no further need for an object. The
> garbage collector
> > 575:      * may reclaim an object even if values from that object's
> fields are still
> > 576:      * in use, or while a method on the object is still running, so
> long as the
>
> Suggestion: method _of_ the object
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 579:
>
> > 577:      * object has otherwise become unreachable.
> > 578:      * <p>
> > 579:      * This may have surprising and undesirable effects, in
> particular when using
>
> Nit: I don't think a new paragraph is appropriate here as "This may ..."
> refers back to the subject of the preceding lines.
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 581:
>
> > 579:      * This may have surprising and undesirable effects, in
> particular when using
> > 580:      * a Cleaner or finalizer for cleanup. If an object becomes
> unreachable while
> > 581:      * a method on the object is running, it can lead to a race
> between the
>
> Again suggest: method _of_ the object
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 585:
>
> > 583:      * Cleaner or finalizer. For instance, the cleanup thread could
> cleanup the
> > 584:      * resource, followed by the program thread (still running the
> method)
> > 585:      * attempting to access the now-already-freed resource.
>
> suggestion: s/cleanup/free/ to match the `now-already-freed` part.
>
> src/java.base/share/classes/java/lang/ref/Reference.java line 586:
>
> > 584:      * resource, followed by the program thread (still running the
> method)
> > 585:      * attempting to access the now-already-freed resource.
> > 586:      * {@code reachabilityFence} can prevent this race by ensuring
> that the
>
> Suggestion: `Use of {@code reachabilityFence ...`
>
> This avoids starting a sentence with a code font word that starts with a
> lower-case letter.
>
> -------------
>
> PR Review Comment:
> https://git.openjdk.org/jdk/pull/16644#discussion_r1534946886
> PR Review Comment:
> https://git.openjdk.org/jdk/pull/16644#discussion_r1534947996
> PR Review Comment:
> https://git.openjdk.org/jdk/pull/16644#discussion_r1534948233
> PR Review Comment:
> https://git.openjdk.org/jdk/pull/16644#discussion_r1534948894
> PR Review Comment:
> https://git.openjdk.org/jdk/pull/16644#discussion_r1534949965
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20240321/c956dca0/attachment-0001.htm>