finalize() called on strongly reachable objects in Java 8

Question

We recently upgraded our message processing application from Java 7 to Java 8. Since the upgrade, we get an occasional exception that a stream has been closed while it is being read from. Logging shows that the finalizer thread is calling finalize() on the object that holds the stream (which in turn closes the stream).

The basic outline of the code is as follows:

MIMEWriter writer = new MIMEWriter( out );
in = new InflaterInputStream( databaseBlobInputStream );
MIMEBodyPart attachmentPart = new MIMEBodyPart( in );
writer.writePart( attachmentPart );

MIMEWriter and MIMEBodyPart are part of a home-grown MIME/HTTP library. MIMEBodyPart extends HTTPMessage, which has the following:

public void close() throws IOException
{
    if ( m_stream != null )
    {
        m_stream.close();
    }
}

protected void finalize()
{
    try
    {
        close();
    }
    catch ( final Exception ignored ) { }
}

The exception occurs in the invocation chain of MIMEWriter.writePart, which is as follows:

1. MIMEWriter.writePart() writes the headers for the part, then calls part.writeBodyPartContent( this )
2. MIMEBodyPart.writeBodyPartContent() calls our utility method IOUtil.copy( getContentStream(), out ) to stream the content to the output
3. MIMEBodyPart.getContentStream() just returns the input stream passed into the contstructor (see code block above)
4. IOUtil.copy has a loop that reads an 8K chunk from the input stream and writes it to the output stream until the input stream is empty.

The MIMEBodyPart.finalize() is called while IOUtil.copy is running, and it gets the following exception:

java.io.IOException: Stream closed
    at java.util.zip.InflaterInputStream.ensureOpen(InflaterInputStream.java:67)
    at java.util.zip.InflaterInputStream.read(InflaterInputStream.java:142)
    at java.io.FilterInputStream.read(FilterInputStream.java:107)
    at com.blah.util.IOUtil.copy(IOUtil.java:153)
    at com.blah.core.net.MIMEBodyPart.writeBodyPartContent(MIMEBodyPart.java:75)
    at com.blah.core.net.MIMEWriter.writePart(MIMEWriter.java:65)

We put some logging in the HTTPMessage.close() method that logged the stack trace of the caller and proved that it is definitely the finalizer thread that is invoking HTTPMessage.finalize() while IOUtil.copy() is running.

The MIMEBodyPart object is definitely reachable from the current thread's stack as this in the stack frame for MIMEBodyPart.writeBodyPartContent. I don't understand why the JVM would call finalize().

I tried extracting the relevant code and running it in a tight loop on my own machine, but I cannot reproduce the problem. We can reliably reproduce the problem with high load on one of our dev servers, but any attempts to create a smaller reproducible test case have failed. The code is compiled under Java 7 but executes under Java 8. If we switch back to Java 7 without recompiling, the problem does not occur.

As a workaround, I've rewritten the affected code using the Java Mail MIME library and the problem has gone away (presumably Java Mail doesn't use finalize()). However, I'm concerned that other finalize() methods in the application may be called incorrectly, or that Java is trying to garbage-collect objects that are still in use.

I know that current best practice recommends against using finalize() and I will probably revisit this home-grown library to remove the finalize() methods. That being said, has anyone come across this issue before? Does anyone have any ideas as to the cause?

Danit

I would be surprised if there is no other explanation to this. The current thread is always a "root" for the collector to identify live objects. How do you know for sure that the finalizer is being invoked before your IOUtils.copy() returns ? – Elroy

This sounds very much like a JIT bug. I would run with JIT debugging turned on and see whether there's any pattern there. – Rinehart

@Bhaskar, the exception proves that the stream is closed while IOUtil.copy() is executing. – Danit

@chrylis, which flags would you suggest turning on? Is it just -XX:+PrintCompilation to try to see if the occurrence of the problem aligns with a JIT compilation of one of the methods? – Danit

Is there a second, unreachable MIMEBodyPart holding a reference to the same m_stream object? – Woe

@Woe that's a good thought, but no there is only one MIMEBodyPart that the stream in is given to. – Danit

Update: finalize is now deprecated, as of Java 9. See the Question, Why is the finalize() method deprecated in Java 9?. – Bil

Answer 1

A bit of conjecture here. It is possible for an object to be finalized and garbage collected even if there are references to it in local variables on the stack, and even if there is an active call to an instance method of that object on the stack! The requirement is that the object be unreachable. Even if it's on the stack, if no subsequent code touches that reference, it's potentially unreachable.

See this other answer for an example of how an object can be GC'ed while a local variable referencing it is still in scope.

Here's an example of how an object can be finalized while an instance method call is active:

class FinalizeThis {
    protected void finalize() {
        System.out.println("finalized!");
    }

    void loop() {
        System.out.println("loop() called");
        for (int i = 0; i < 1_000_000_000; i++) {
            if (i % 1_000_000 == 0)
                System.gc();
        }
        System.out.println("loop() returns");
    }

    public static void main(String[] args) {
        new FinalizeThis().loop();
    }
}

While the loop() method is active, there is no possibility of any code doing anything with the reference to the FinalizeThis object, so it's unreachable. And therefore it can be finalized and GC'ed. On JDK 8 GA, this prints the following:

loop() called
finalized!
loop() returns

every time.

Something similar might be going on with MimeBodyPart. Is it being stored in a local variable? (It seems so, since the code seems to adhere to a convention that fields are named with an m_ prefix.)

UPDATE

In the comments, the OP suggested making the following change:

    public static void main(String[] args) {
        FinalizeThis finalizeThis = new FinalizeThis();
        finalizeThis.loop();
    }

With this change he didn't observe finalization, and neither do I. However, if this further change is made:

    public static void main(String[] args) {
        FinalizeThis finalizeThis = new FinalizeThis();
        for (int i = 0; i < 1_000_000; i++)
            Thread.yield();
        finalizeThis.loop();
    }

finalization once again occurs. I suspect the reason is that without the loop, the main() method is interpreted, not compiled. The interpreter is probably less aggressive about reachability analysis. With the yield loop in place, the main() method gets compiled, and the JIT compiler detects that finalizeThis has become unreachable while the loop() method is executing.

Another way of triggering this behavior is to use the -Xcomp option to the JVM, which forces methods to be JIT-compiled before execution. I wouldn't run an entire application this way -- JIT-compiling everything can be quite slow and take lots of space -- but it's useful for flushing out cases like this in little test programs, instead of tinkering with loops.

Anatomical

Thanks @Stuart Marks - you've restored my faith in Stack Overflow. The interesting think about your program is that it gives me the same on Java 7 and Java 8. My issue only appeared when we switched to Java 8. Your analysis makes sense though, and further inspires me to remove finalize() from our code base. – Danit

Another query - I changed the main method in your program to FinalizeThis finalizeThis = new FinalizeThis(); finalizeThis.loop();. Once I did that, the object was never finalized. I'm not sure why that would be different. Do you have any ideas? – Danit

@Danit I've updated my answer in response to your comment. Not sure about why you only see the problem in your app on Java 8. A bunch of JIT heuristics probably changed between 7 and 8, which might be responsible for behavior differences like this. Also, if you remove the finalizer, make sure that something eventually closes the stream. Maybe use try-with-resources. – Anatomical

Since finalization can be performed by any thread, theoretically the output could be the result of a race condition without any time relationship between the loop and the finalization. I suggest to let the finalize method modify a static volatile boolean variable and the loop reading that variable so it will be even the loop itself detecting its own this finalization while running (and it might break out of the loop when detecting the finalization, so we don’t have to wait for the program termination more than necessary). – Scare

Another remark: if the JVM determines that “no subsequent code touches that reference” and finalizes it, it’s a clear sign that the caller has forgotten to call the close() method, isn’t it? After all, invoking the close() method implies touching the reference, so the finalize method fulfills its purpose: closing what has forgotten to be closed (only a bit too early)… – Scare

@Scare Yes having the finalizer perform a side effect that can be awaited is a time-honored technique. Re closing the stream, the OP's app apparently has some calling code create the stream, which is then passed to the MIMEBodyPart constructor and stored in a field. It's the MIMEBodyPart instance that becomes unreachable and is finalized, it closes the stream. The calling code still has a reference to the stream and finds it closed when it tries to use it. The conclusion is that the MIMEBodyPart finalizer shouldn't close the stream because it didn't open it. – Anatomical

@Stuart Marks: The class MIMEBodyPart has a finalize() and a close() method as seen in the question. And finalize() doesn’t call close() on the stream directly but via its own instance method close() which will then invoke close() on the stream. So it’s seems that the purpose is that close() shall be invoked on the MIMEBodyPart instance and finalize() help out if close() has not been called. And that seems to be the case here, the close() method of the MIMEBodyPart instance has not been called as otherwise it couldn’t become garbage collected before the call. – Scare

There's a possibility that the compiler has changed the void loop() method into a static method since it does not reference any instance variable or method. If you have a refrence to the FinalizeThis object in void loop(), it won't be GCed. – Gauss

Another finding: If you set a breakpoint at System.out.println("loop() returns"); and debug run it, it won't be GCed. – Gauss

If you really need to keep an object alive until a certain point is reached, then java.lang.ref.Reference.reachabilityFence(obj) will do exactly that. – Dolph

Answer 2

finalize has 99 problems, and premature finalization is a new one.

Java 9 has introduced Reference.reachabilityFence to work around this issue. The documentation also mentions using synchronized (obj) { ... } as an alternative on Java 8.

But the real solution is to not use finalize.

Selfsufficient

Answer 3

Your finalizer isn't correct.

Firstly, it doesn't need the catch block, and it must call super.finalize() in its own finally{} block. The canonical form of a finalizer is as follows:

protected void finalize() throws Throwable
{
    try
    {
        // do stuff
    }
    finally
    {
        super.finalize();
    }
}

Secondly, you're assuming you are holding the only reference to m_stream, which may or may not be correct. The m_stream member should finalize itself. But you don't need to do anything to accomplish that. Ultimately m_stream will be a FileInputStream or FileOutputStream or a socket stream, and they already finalize themselves correctly.

I would just remove it.

Xerophagy

I agree in principle about calling super.finalize() but in this case HTTPMessage extends Object which has an empty finalize(). I agree that the code as it stands isn't optimal, but I'm not sure that's relevant either. The problem is that finalize() seems to be called incorrectly under Java 8 but it wasn't under Java 7. – Danit

I would still remove it and see what happens. You don't need it and it is a possible source of error for all the reasons I stated. – Xerophagy

I agree that the finalize() is not needed in this case (we call in.close() directly further down in the code). However, our HTTP/MIME library is used in other scenarios where finalize() may be needed. As I mentioned, I will revisit the library and all its uses in the future to remove finalize(). I have also already rewritten the code in question to avoid the problem. We have other areas of our code that also have finalize() methods. My concern is that we might run into this same issue in other parts of our code. – Danit

Is it acceptable to call super.finalize() first, or must it only be called after whatever this finalizer needs to do? – Celsacelsius

@DavidConrad You might get lucky, but why? Resources should be released in the reverse order of allocation. – Xerophagy

@EJP I was thinking to avoid the try/finally, but you're right about the order of course. I see that now. Thanks. – Celsacelsius