CODEWITHSUNDEEP
javastream
VdS
VdS

Reputation: 47

Java, closing inline streams

Edit: forgot to mention I'm using java 6

I was wondering about how to close resources in java.

See, I always have initialized streams like this:

ZipInputStream zin = null;

try {
    zin = new ZipInputStream(new BufferedInputStream(new FileInputStream(file)));
    // Work with the entries...
    // Exception handling
} finally {
    if (zin!=null) { try {zin.close();} catch (IOException ignored) {} }
}

But, if an exception is thrown in new ZipInputStream(...), would the opened streams in new BufferedInputStream and underliying FileInputStream be leaked?

If they are, what would be the most efficient way to ensure the resources are closed?, should I have to keep a reference to each new ...Stream and close them also in the finally block?, or should the final stream (ZipInputStream in this case) instantiated in some other way?.

Any comments are welcome.

Upvotes: 1

Views: 1841

Answers (4)

aioobe
aioobe

Reputation: 421310

You can do

try (InputStream s1 = new FileInputStream("file");
     InputStream s2 = new BufferedInputStream(s1);
     ZipInputStream zin = new ZipInputStream(s2)) {
    // ...
} catch (IOException e) {
    // ...
}

Further reading: The Java™ Tutorials: The try-with-resources Statement.

Upvotes: 3

Durandal
Durandal

Reputation: 20069

First lets take a look at what you have and what can go wrong with it:

try {
    zin = new ZipInputStream(new BufferedInputStream(new FileInputStream(file)));
    // Work with the entries...
    // Exception handling
} finally {
    if (zin!=null) { try {zin.close();} catch (IOException ignored) {} }
}

a.) new FileInputStream() throws, zin will not be assigned. Nothing to close in this case. Ok.
b.) new BufferedInputStream() throws (possibly OutOfMemoryError), zin not assigned. Leaked FileInputStream(). Bad.
c.) new ZipInputStream() throws, zin will not be assigned. BufferedInputStream and FileInputStream to close. Closing either would be enough. Bad.

Whenever you wrap one stream into another, you are in danger of leaking the stream youre wrapping. You need to have a reference to it and close it somewhere.

A viable way top this is to declare a single InputStream variably to hold the last create stream (or in other words, the outermost of the nested streams):

InputStream input = null;
try {
    input = new FileInputStream(...);
    input = new BufferedInputStream(input);
    input = new ZipInputStream(input);
    ZipInputStream zin = (ZipInputStream) input;
    // work here
} finally {
     if (input != null)
         try { input.close(); } catch (IOException ignored) {}
}

This works, because if any of the new *Stream() throws, the variable input is still keeping track of the stream created before. The ugly cast from input to ZipInputStream is necessary, because you must declare input to be a type assignment compatible to all streams created.

Upvotes: 0

Thomas Stets
Thomas Stets

Reputation: 3043

Yes, an Exception in new ZipInputStream() or new BufferedInputStream() would leak the enclosed Streams, unless you do a cascading check in the exception handling:

FileInputStream fin = null;
BufferedInputStream bin = null;
ZipInputStream zin = null;

try {
    fin = new FileInputStream(file);
    bin = new BufferedInputStream(fin)
    zin = new ZipInputStream(bin);
    // Work with the entries...
    // Exception handling
} finally {
  try {
    if (zin!=null) {
       zin.close();
     } else if (bin != null) {
       bin.close();
     } else if (fin != null) {
       fin.close();
     }
  } catch (Exception e) {
    // ignore
  }
}

However, since BufferedInputStream and ZipInputStream are mere wrapper around the FileInputStream the probability of an Exception is rather low. If at all, an Exception if most likely to happen once you start reading and processing data. And in that case zin is created, and a zin.close() will suffice.

Upvotes: 0

Yuriy Semen
Yuriy Semen

Reputation: 92

It can be done in this way:

BufferedInputStream bis = new BufferedInputStream(new FileInputStream(file));
try {
    ZipInputStream zin = new ZipInputStream(bis);
    try {
        zin = ;
        // Work with the entries...
        // Exception handling
    } finally {
        zin.close();
    }
} finally {
    bis.close();
}

And you can add error caching where you want.

Upvotes: 1

Related Questions