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

[Andrew Haley]

On 10/10/2018 11:22 AM, Luke Hutchison wrote:
> On Wed, Oct 10, 2018 at 3:50 AM Andrew Haley <aph at redhat.com> wrote:
> 
>>> This makes finalizers a lot less reliable and more difficult to use
>>> than they could be.
>>>
>>> Wouldn't it make sense to consider the invocation targets in the stack
>>> during liveness analysis, so that out of the cases you gave, "finalizers
>>> may be run too soon (as here,) too late, or never", at least the "too
>> soon"
>>> case is fixed?
>>
>> We considered doing that; the discussion is online somewhere. I was in
>> favour of doing what you suggest, but there is some performance impact
>> of preserving liveness for all methods. In the end it was decided that
>> we'd provide reachabilityFence.
> 
> The stack already has to be walked for reachability analysis, and there
> would no doubt be some performance impact for marking invocation targets as
> reachable, but that already has to be done for a conceivably much larger
> set of objects than just the number of stack frames, so it seems like the
> overall impact to performance would be minimal.

Not necessarily. A JIT compiler has a great deal of latitude to
reorder operations, and if it can see that the only access to an
Object O before it dies is to read Field X, it can generate code to
read X, cache it somewhere, and delete the last live reference to O.

> This is especially true given that only classes that override
> finalize() would need to be tracked in this way. Was this
> benchmarked before this decision was made, in order to determine
> actual rather than theorized performance impact?

I don't know. I do know that it would increase the lifetime of
temporaries, and thereby increase pressure on the register
allocator. It would have been interesting to change the Java
specification to do that, but it would be difficult.

> I don't know what went into the discussion you refer to, but not
> doing what is arguably "the right thing" in the name of what is
> probably a tiny performance margin doesn't seem strongly
> justifiable. I understand your arguments about elision of methods
> etc., but I really can't believe that a currently running method
> would not be considered a strong enough reference to "this".

Believe it. It is true.

We have been dicussing problems with finalization for more than a
decade, and almost every possible argument has been made.
reachabilityFence was a great step forward.

> Forcing users to put reachabilityFence wrappers (or "synchronized
> (this)" wrappers) around every single method of every object that
> contains a finalizer seems like a very poor fix to this problem.
> It is well known that a lot of things about finalizers are
> broken. But why not make them a little less broken?

Again, because it would increase the lifetime of temporaries.

>> You need two threads to have a race condition. I'm not sure what
>> you're trying to ask: the reachabilityFence must run strictly after
>> the body of the try clause, and it the object will be reachable until
>> then.
> 
> Right -- one thread is the garbage collector.
> 
> What I'm asking is with a method invocation "new A().someMethod()", is
> there conceivably a gap in time between when the runtime realizes that
> there are no more references to the "A" object in the caller, and when it
> realizes that there is a fenced reference to "this" in "someMethod"?

No, there cannot be. "this" has to be kept alive because it's an input
to the reachabilityFence.

> I'm guessing the answer is no, otherwise the reachabilityFence
> mechanism would not always work, but I was asking about the exact
> ordering of the dropping of the invocation target reference, and
> adding object references within the invoked method to the set of
> reachable objects.

"this" is an input to the reachabilityFence, and the fence cannot be
moved. It's important to realize that reachabilityFence is magic and
is known to the compilers. It's similar to a volatile store of "this"
but in some ways even stronger, and with less runtime overhead.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671