RFR: 8283660: Convert com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java finalizer to Cleaner [v8]

[Hans Boehm]

On Wed, Jun 1, 2022 at 2:47 PM Roger Riggs <rriggs at openjdk.java.net> wrote:

> ...
> > 214:         } finally {
> > 215:             // Ensure Cleaner does not run until after this method
> completes
> > 216:             Reference.reachabilityFence(this);
>
> I don't think there is any benefit to the `try{} finally {fence}`.
> The reachabilityFence has no executable code. Its only purpose is to keep
> the reference in scope alive.
>
> That's an interesting general question.

I agree that this is true with the right implementation assumptions, which
might conceivably be warranted here. But if there is a possibility that the
block throws after a point at which you need this to ensure reachability,
then I don't think this is spec-correct without the try-finally.  Consider

  tmp = a.field;
  use(field);  // Cleaner associated with a invalidates field
  if (moon_phase() == FULL) throw(...);
  Reference.reachabilityFence(a);

Consider the full moon case. The reachabilityFence spec says: "the
referenced object is not reclaimable by garbage collection at least until
after the invocation of this method." This method is not invoked, so there
is no guarantee, and hence this may fail.

And indeed, a compiler could conceivably rewrite this to

  if (moon_phase() != FULL) {
    tmp = a.field;
    use(field);  // Cleaner associated with a invalidates field
    Reference.reachabilityFence(a);
  } else {
    tmp = a.field;
    use(field);  // Cleaner associated with a invalidates field <--
potential crash
    throw(...);
  }

in which case this might, on very rare occasions, actually fail in the
throwing case, since the reference a may not be kept in the else clause.

Hans


> PR: https://git.openjdk.java.net/jdk/pull/8311
>