Should streams be closed after every use?

Question

I am currently working on a UDP server for a game. In this server use a ByteArrayInputStream and a ObjectInputStream every tick to convert serialized bytes to objects. Is it more efficient to create one variable for the streams and close them once when the program is closing?

like so:

class Main {
private static ByteArrayInputStream byteIn;
private static ObjectInputStream objectIn;

public static void main(String[] args) {
    while(true){
      receive();
    }
    //when program is done call close();
}

 public static void receive(){
     byteIn = new ByteArrayInputStream();
     objectIn = new ObjectInputStream(new BufferedInputStream(byteIn));
      //do something
  }

 public static void close(){
    objectIn.close();
    byteIn.close();
  }
}

Or is it more efficient create and close new streams every time?

like so:

class Main {

public static void main(String[] args) {
    while(true){
      receive();
    }
}

 public static void receive(){
    ByteArrayInputStream byteIn = new ByteArrayInputStream();
    ObjectInputStream objectIn = new ObjectInputStream(new BufferedInputStream(byteIn));
    //do something
    objectIn.close();
    byteIn.close();
  }
}

In both examples you'll be creating millions of input streams every second thanks to that infinite while loop — Michael, Mar 25 '19 at 13:15

score 2 · Accepted Answer · answered Mar 25 '19 at 13:17

If you open a stream, you should close it. The code in the question doesn't do that, it just abandons previous streams and closes only the very last ones it creates.

It's not clear why those stream variables should be static rather than local within receive. If they were local within receive, you could use try-with-resources to automatically clean them up:

public static void receive(){
    try (
        ByteArrayInputStream byteIn = new ByteArrayInputStream();
        ObjectInputStream objectIn = new ObjectInputStream(new BufferedInputStream(byteIn));
    ) {
        //do something
    }
}

They'll be closed automatically when control passes out of the try.

If they have to be static class members for some reason, just close and release each one you create (but it's a lot easier to have bugs in your code that means it has execution paths that fail to do that).

// All done
objectIn.close();
objectIn = null;
byteIn.close();
byteIn = null;

Side note: With those three stream types, you may not need byteIn as a separate variable. More in this question's answers.

score 2 · Answer 2 · answered Mar 25 '19 at 13:17

2

Yes you need to close the streams. Use a try-with-resources block. It will close the stream for you.

answered Mar 25 '19 at 13:17

Nicholas K

15,148
7
31
57

Should streams be closed after every use?

2 Answers2