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

[Peter B. Kessler]

The JavaDoc for `Reference.reachabilityFence(Object ref)` [http://hg.openjdk.java.net/jdk/jdk/file/62523934374c/src/java.base/share/classes/java/lang/ref/Reference.java#l509] says

	@param ref the reference. If {@code null}, this method has no effect.

It might be true that if the actual parameter is a constant `null` the method has no effect.  I understand the intent of the method if applied to a simple variable (e.g., `this`).  What is the effect of calling the method with a complex expression that yields an `Object`?  How does the compiler know which lifetime to extend?  When is the actual parameter evaluated?  What if the complex expression yields `null`?  Does the method still have "no effect"?

The try-with-resource solution also operates on expressions, but there it is more obvious what to make a strong reference to to keep a particular Object reachable.

Hans's `@Cleaned` annotation can only be applied to fields, not expressions.  Does that make it less useful?

			... peter

On 10/11/18 12:56 PM, Hans Boehm wrote:
> 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.
>