Finalizer being run while class still in use (escape analysis bug)

[Hans Boehm]

On Wed, Oct 10, 2018 at 10:42 PM Erik Osterlund <erik.osterlund at oracle.com>
wrote:
>
> Hi Hans,
>
> Would users be happier if e.g. try with resources could be relied upon
having things reachable in a scope for this pattern?
>
> try (SlipperyObject x = getSlipperyObject()) {
>   // x is strongly reachable in this scope
> }
> // x won’t die until here
>
> Seems more intuitive for me to reason about the reachability in a scope
than manually analysing the uses of locals.
>
> I suppose you would have to implement AutoCloseable to be allowed to use
such a construct. But a SlipperyObject class could maybe wrap that and even
call reachability fence in its close method for portability.
>
> The trailing close function might already have the same effect as
reachabilityFence.
>
> Just an idea.
>
> Thanks,
> /Erik
>

I think the easiest and most common use case is one in which a class C
includes a field that needs to be explicitly cleaned up when an instance of
C is dropped.
(Unfortunately even libcore contains a bunch of more complicated patterns,
but let's ignore those here.)

In that case, the alternatives are basically:

Current:

class C {
  private long f; // A handle to some resource needing explicit cleaning.
  ...
  Foo someMethod(C that) {
    try {
      ... access f and that.f ...
    } finally {
      reachabilityFence(this);
      reachabilityFence(that);
   }
   ... repeat for other methods ...
}

IIUC, with your proposal, it becomes

class C {
  private long f; // A handle to some resource needing explicit cleaning.
  ...
  Foo someMethod(C that) {
    try (C me = this; c myThat = that) {
      ... access f and that.f, or equivalently me.f and myThat.f ...
   }
   ... repeat for other methods ...
}

That's clearly shorter by a constant factor. But to me it doesn't look
consistent with the normal try-with-resources statement.

My alternative proposal would reduce this to:

class C {
  @Cleaned private long f; // A handle to some resource needing explicit
cleaning.
  ...
  Foo someMethod(C that) {
        ... access f and that.f ...
  }
   ... Other methods also need no special treatment ...
}

This has the advantage that the annotation is confined to one place,
arguably where it belongs. The annotation overhead is shorter by a factor
of O(<number of methods>). It doesn't get around the fact that this is
really difficult to explain to a non-compiler-expert in full detail. But I
think it's possible to quickly explain the annotation in a way that will
make sense to most people and will generally lead them to correct code.

Full disclosure: This remains messy in some cases, notably if you let f
escape from C by handing it to a caller. There's some argument that that
should never happen for a properly designed API. It puts the caller into
the position of explicitly having to reason about object lifetimes.
Unfortunately things like this do seem to be hard to avoid ins some cases.
Even in the messy cases, so far I've almost always found @Cleaned to be
more convenient than explicit reachabilityFences.