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

[Erik Osterlund]

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

> On 11 Oct 2018, at 01:58, Hans Boehm <hboehm at google.com> wrote:
> 
> From: Luke Hutchison <luke.hutch at gmail.com>
> 
>> On Wed, Oct 10, 2018 at 9:53 AM Kim Barrett <kim.barrett at oracle.com> wrote:
>> Finalization was deprecated in JDK 9 (*).  There's an ongoing (though
>> progressing slowly) process of eliminating its usage in the JDK.
>> Sometime after that it may go away entirely, or perhaps require some
>> sort of explicit opt-in, making it possible to eliminate various GC
>> overheads associated with it.
>> 
>> (*) Use PhantomReference, or a helper like java.lang.ref.Cleaner.
>> Though that change the liveness question in this thread.
>> 
> Yes, I saw both the deprecation of finalization and the introduction of
> Cleaner -- though there will be a lot of legacy code and legacy runtime
> environments that are going to stick around for a long time, especially
> given the problems so many people are having moving to JPMS for non-trivial
> usecases.
> What would be wrong with simply disabling inlining altogether for any
> classes that override finalize()? Inlining seems to be the main sticking
> point. I would strongly prefer to take a performance hit over having
> finalization being so broken. Running finalizers late or never is much less
> of a problem than running them early, since running them late or never
> *may* lead to resource leaks, but running them early *will almost
> certainly* break everything that uses finalizers at some point.
> It would be a much better API contract to state that "if you override
> finalize(), some code optimizations may not kick in" rather than "if you
> override finalize(), things will almost certainly break, and you get to
> keep all the pieces".
> 
> 
> I don't think inlining is the real problem. The general problem is that
> just because a field
> of an object is still in use, does not mean that the containing object is
> still reachable.
> The register holding the reference may have been reused. e.g. for the field
> value itself.
> The receiver reference inside a method is really just like any other
> parameter. And it's
> common to pass parameters in registers that can be reused once we no longer
> need
> the parameter values, e.g. because we've retrieved the fields we need.
> 
> One could solve this by preventing elimination of dead references. But,
> it's hard to do this
> only where it matters. Doing it everywhere is an option, but hasn't gotten
> a lot of traction.
> There is a more elaborate proposal to make this easier than
> reachabilityFence at
> https://docs.google.com/document/d/1yMC4VzZobMQTrVAraMy7xBdWPCJ0p7G5r_43yaqsj-s/edit?usp=sharing
> 
> Currently reachabilityFence is by far the best solution. (Passing the
> reference to an
> existing native method, e.g. by removing "static" is more subtle, but
> otherwise as good when
> it works.) I believe the cost of reachabilityFence in implementations that
> support it is near zero; all it has to do is keep a value around on the
> stack or in a register.
> In my experience, it's quite hard to portably, efficiently, and robustly
> implement reachabilityFence()
> where it's not already supported. Passing it to another Java method may
> work, but is not
> portable.
> 
> ReachabilityFence still has the substantial disadvantage that it often
> needs to be used in many methods after referring to a field that is cleaned
> up by a Cleaner or finalizer, etc. The
> above proposal allows the field declaration to be modified, instead of
> every method that
> touches it. But the programmer still needs to know that there is an issue.
> (@ReachabilitySensitive is a placeholder name. @Cleaned is the current
> front runner in
> my vicinity.)
> 
> Hans