CODEWITHSUNDEEP
javaiotry-with-resources
Nathan
Nathan

Reputation: 1478

Java 8 FilterOutputStream exception

The was a change to the FilterOutputStream.close() method in Java 8 that is causing us some problems. (See http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/759aa847dcaf)

In previous versions of Java, the following code worked without throwing an exception. However, under Java 8 we always get an exception when the try-with-resources mechanism closes the streams.

try( InputStream bis = new BufferedInputStream( inputStream );
     OutputStream outStream = payloadData.setBinaryStream( 0 );
     BufferedOutputStream bos = new BufferedOutputStream( outStream );
     DeflaterOutputStream deflaterStream = new DeflaterOutputStream(
         bos, new Deflater( 3 ) ) )
{
    fileSize = IOUtil.copy( bis, deflaterStream );
}

The try-with-resources mechanism will first call close() on deflaterStream. Since deflaterStream wraps bos which wraps outStream, deflaterStream.close() will call bos.close() which will call outStream.close() which closes the underlying stream to the database.

The try-with-resources mechanism will next call close() on bos. Since bos extends FilterOutputStream, flush() will first be called on outStream. However, since outStream is already closed, outStream.flush() throws exception: java.sql.SQLException: Closed LOB

caused by: java.io.IOException: Closed LOB
     at oracle.jdbc.driver.OracleBlobOutputStream.ensureOpen(OracleBlobOutputStream.java:265)
     at oracle.jdbc.driver.OracleBlobOutputStream.flush(OracleBlobOutputStream.java:167)
     at java.io.BufferedOutputStream.flush(BufferedOutputStream.java:141)
     at java.io.FilterOutputStream.close(FilterOutputStream.java:158)
     at com.blah.uploadFile(CustomerUploadFacade.java:162)
     ... 38 more
Caused by: java.sql.SQLException: Closed LOB
     at oracle.jdbc.driver.OracleBlobOutputStream.ensureOpen(OracleBlobOutputStream.java:257)
     ... 42 more

Has anyone else experienced this issue? If so how did you work around it? Is there something wrong with the way we're using try-with-resources?

Upvotes: 4

Views: 1908

Answers (4)

Nathan
Nathan

Reputation: 1478

Our final solution was to fix the FilterOutputStream ourselves:

public void close() throws IOException {
    if (!closed) {
        closed = true;
        try (OutputStream ostream = out) {
            flush();
        }
    }
}

private boolean closed = false;

This class was included in a small JAR file and added to the boot classpath: -Xbootclasspath/p:%APP_HOME%\lib\jdkFix.jar

Hopefully Oracle will acknowledge this issue is a bug and fix it in a future update of Java 8.

Upvotes: 0

WW.
WW.

Reputation: 24311

This is a bug in FilterOutputStream. This implements Closable. The contract for this interface states:

Closes this stream and releases any system resources associated with it. If the stream is already closed then invoking this method has no effect.

So calling .close() a second time should have no effect, but in this case it calls flush() which throws an exception.

FilterOutputStream is stuck in an inheritance hierarchy, so it's not easy to apply a fix at the point of the problem.

Failing to declare some of the streams as local variables in the try block will cause tools like FindBugs and Eclipse to flag the code as resource usage. Diligent developers looking at this code later might think the same thing.

You could create a class, CloseOnceBufferedOutputStream which extends BufferedOutputStream. It will have a boolean to remember if it has already been closed so it can meet the contract. Override the close() method to check if the stream is already closed. If so, just return.

If and when Oracle fix the underlying bug, your new class would be useless but the code would continue to work.

Upvotes: 2

icza
icza

Reputation: 418357

Simply don't declare bos and outStream in the try and so they won't be automatically closed. Only the declared AutoClosables will be closed.

The JVM does not analyze the declared AutoClosables whether one is constructed using another, so each of them must be closed explicitly. If their close() method can only be called once, that could lead to problems you're experiencing.

Only declare deflaterStream like this:

try( InputStream bis = new BufferedInputStream( inputStream );
     DeflaterOutputStream deflaterStream = new DeflaterOutputStream(
        new BufferedOutputStream( payloadData.setBinaryStream( 0 ) ),
            new Deflater( 3 ) ) )
{
    fileSize = IOUtil.copy( bis, deflaterStream );
}

Edit:

Following the comments here's how you could close the stream returned by payloadData.setBinaryStream( 0 ) in case of some other issue:

OutputStream outStream = null;
try( InputStream bis = new BufferedInputStream( inputStream );
     DeflaterOutputStream deflaterStream = new DeflaterOutputStream(
        new BufferedOutputStream( outStream = payloadData.setBinaryStream( 0 ) ),
            new Deflater( 3 ) ) )
{
    fileSize = IOUtil.copy( bis, deflaterStream );
}
catch (ExceptionsYouWantToCatch eywtc)
{
    if (outStream != null) {
        // Here you have the chance to close it
        try { outStream.close(); } catch(IOException ie){}
    }
}

Upvotes: 1

Peter Lawrey
Peter Lawrey

Reputation: 533730

IMHO, You shouldn't have to do this but...

You don't need to declare all output streams you create, instead you can just declare the outer most ones.

try( InputStream bis = new BufferedInputStream( inputStream );
     DeflaterOutputStream deflaterStream = new DeflaterOutputStream(
         new BufferedOutputStream( payloadData.setBinaryStream( 0 ) ), new Deflater( 3 ) ) )
{
    fileSize = IOUtil.copy( bis, blobDeflaterStream );
}

This way deflatorStream will be closed, which will close the others.

Upvotes: 0

Related Questions