InputStream Wrapper that closes the stream once reading is done

Question

I faced some issues in a project with spring and hibernate where the stream given to Blob is not closed.

Blob blob = Hibernate.getLobCreator(currentSession).createBlob(inputStream, size);

I can't use things like try with resources since the stream is only read after the transaction commits. One of my friends suggested a hacky way to do this. Basically, his idea was to close the stream once the reading is complete.

import java.io.IOException;
import java.io.InputStream;

public class InputStreamWrapper extends InputStream {

    private InputStream inputStream;

    public InputStreamWrapper(InputStream inputStream) {
        this.inputStream = inputStream;
    }

    @Override
    public int read() throws IOException {
        int val = this.inputStream.read();
        if (val != -1) {
            return val;
        } else {
            this.inputStream.close();
            return -1;
        }
    }
}

I've done a simple test and it works. I understand that there can be a performance issue with the if check. However, I think there are other caveats in this as well. Can someone shed some light on those?

Blob blob = Hibernate.getLobCreator(currentSession).createBlob(new InputStreamWrapper(inputStream), size);

Answer 1

You should also implement int read(byte[] b, int off, int len), delegating it to inputStream.read(b, off, len).

@Override
public int read(byte[] b, int off, int len) throws IOException {
    try {
        int val = inputStream.read(b, off, len);
        if (val == -1)
           inputStream.close();
        return val;
    } catch (IOException e) {
        inputStream.close();
        throw e;
    }
}

Without this, all bulk operation will end up being processed byte by byte, with several call/return stack frames for each byte.

Note: You don’t need to override the other bulk read operations, as they already delegate to this one. Ie, read(byte[] b) is implemented as read(b, 0, b.length), so will end up using the above code.

---

Actually, there are a few more things wrong with your wrapper. You've lost the implementations for available(), close(), mark(), markSupported(), reset() and skip()!

Instead of extending InputStream where you will need to provide the implementation for all of these methods yourself, you should instead extend FilterInputStream, passing in the original stream to its constructor; it will forward these additional functions to the original stream for you.

public class InputStreamWrapper extends FilterInputStream {

    public InputStreamWrapper(InputStream is) {
        super(is);
    }

    @Override
    public int read() throws IOException {
        try {
            int val = super.read();
            if (val == -1)
                close();
            return val;
        } catch (IOException e) {
            close();
            throw e;
        }
    }

    @Override
    public int read(byte[] b, int off, int len) throws IOException {
        try {
            int val = super.read(b, off, len);
            if (val == -1)
                close();
            return val;
        } catch (IOException e) {
            close();
            throw e;
        }
    }
}

---

Update. After perusing the source code for FileInputStream, ...

public class FileInputStream ... {
    ...

    private native int readBytes(byte b[], int off, int len) throws IOException;

    public int read(byte b[]) throws IOException {
        return readBytes(b, 0, b.length);
    }

    public int read(byte b[], int off, int len) throws IOException {
        return readBytes(b, off, len);
    }

    ...
}

... it becomes clear that you have to override all flavours of the int read(...) methods. The implementation of int read(byte b[]) does not return read(b, 0, b.length), but rather calls the native readBytes(b, 0, b.length) function directly. With just int read(byte[], int, int) overridden to close the stream when -1 is returned, if the caller uses the int read(byte[]) method, and that returns -1, the stream would not be closed.

Answer 2

The general idea of a wrapper stream that closes the inner stream automatically once its end is reached is fine, but your implementation leaves somewhat to be desired.

First, if you're going to wrap a stream, then do a complete job.

• At minimum, do not fail to override close() properly. I understand that you're trying to solve a problem of close() not being called, but you don't know for sure how instances will be used once you hand them off to another component. If the wrapper is closed before the end of the inner stream is reached, then you want the inner stream closed, too.

• It would be a very good idea to delegate the other InputStream methods, too, with the possible exception of read(byte[]). That will provide the full capabilities of the underlying stream to the wrapper's consumer, including, but not limited to, efficient block reads. For this purpose, you should consider inheriting from FilterInputStream instead of directly from InputStream.

Second, you have to consider how to handle exceptions. With a proper close() method on the wrapper, you're not limited to having read close the inner stream when an exception occurs. Nevertheless, that might still be a good strategy.

Third, your wrapper doesn't necessarily provide a complete solution to the problem. No matter what you do, a situation could arise in which the wrapper is abandoned before the inner stream is closed. If you want to attend to that, too, then your best option is to provide a cleanup strategy based on soft, weak, or phantom references. If you don't already know how to do this then now is a good time to learn -- it's a useful skill to have.

---

On the other hand, you should also consider avoiding the non-closure problem differently, by using a stream for which that's a non-issue. ByteArrayInputStream is such a stream. Using that would mean reading the original data into memory yourself, so you would have control over closing that stream. Failing to close a ByteArrayInputStream is no big deal.

Answer 3

Here's a caveat for ya: if this.inputStream.read() throws an exception, the stream will never close.