Unrelease Resource Stream: Sometimes Garbage Collectors Do Not Save Us

The problem of not releasing the resources we use is that over time the server fills up with things to do and is no longer able to process incoming requests, causing a disservice. This may be due to a wrong program architecture in which it's not clear who has to release what, or simply to a conceptual error on the part of the developer that creates problems in a few lines of code.

The worst case is when an attacker finds a way to generate a resource leak by exploiting this lack of management, falling into what everyone knows as Denial Of Service.

Details

Specifically, the unrelease resource stream occurs when a stream is opened, both in input and output, and for some reason it's not closed, so the memory occupied by this stream is not released.

Among these reasons we often find the fact that the program crashes and the error is handled through a catch block but the resource in the finally block is not released, such as:

try {
  [...]
  // If the program crashes, the following method is not called!
  stream.close();
} catch (Exception ex) {
  System.out.println("Exception: " + ex);
} finally {
  // Last things to do, but I actually had to close the stream here
}

Or we simply always trust too much in the Garbage Collector (which works like the AMA, an Italian company for the collection, treatment and disposal of municipal solid waste: sometimes it passes, sometimes not, sometimes late) and we don't worry in the least to release the resources, because “it will pass by to clean”.

Example

In this example we consider a conceptual error by the developer. Let's start with the following piece of Java code:

try {
  httpInputStream = http.getInputStream();
  inputStreamReader = new InputStreamReader(httpInputStream);
  // Here I work with http, which could become null!
  [...]
} catch (Exception ex) {
  System.out.println("Exception: " + ex);
} finally {
  if (http != null) {
    if (httpInputStream != null) {
      httpInputStream.close();
    }
  }
}

The SAST tool used was Fortify SCA, and reported Unreleased Resource: Stream on the httpInputStream variable. This is because in the finally block the closure of that stream is only performed if the http variable is different from null (and obviously if httpInputStream is different from null, to avoid a possible Null Dereference by invoking the close() method).

What is the problem? That if by chance http becomes null for some hidden reason, the stream is not released. But the developer didn't agree, he told me exactly:

But it is passed by reference, if I close http I close the stream too, so if http becomes null it is also null httpInputStream. In Java all objects are passed by reference.

Well no. I'm sorry. Because in this case an object is not passed by reference but a stream is opened. It's not that anything vaguely resembles an object is passed by reference, come on.

But he still wasn't convinced and so I created a little test program to show him what was going on.

Test #01

I wrote a simple class that loads the page of a very famous site and prints the body. It's pretty much like the code block seen above, but it does something if I run it.

import java.net.*;
import java.io.*;

public class URLConnectionReader {
  public static void main(String[] args) throws Exception {
    URL url = new URL("https://www.youporn.com/");
    URLConnection http = url.openConnection();

    InputStream httpInputStream = null;
    InputStreamReader inputStreamReader = null;

    try {
      httpInputStream = http.getInputStream();

      // Here I work with http, which could become null!
      [...]

      inputStreamReader = new InputStreamReader(httpInputStream);
      StringBuilder stringBuilder = new StringBuilder();
      char[] charBuffer = new char[2048];
      int num;

      while ((num = inputStreamReader.read(charBuffer)) != -1) {
        stringBuilder.append(charBuffer, 0, num);
      }

      System.out.println(stringBuilder.toString());
    } catch (Exception ex) {
      System.out.println("Exception: " + ex);
    } finally {
      if (http != null) {
        if (httpInputStream != null) {
          httpInputStream.close();
        }
      }
    }
  }
}

This code is wrong. As I said earlier, checking if http is non-null makes no sense to release httpInputStream. Also, the InputStreamReader is not closed, but we'll talk about this later.

Ok, I need evidence, so I modified the code by deliberately setting http to null and inserting several println to understand what is happening (obviously in debug it's easier, but so I had the evidence to give to the developer).

import java.net.*;
import java.io.*;

public class URLConnectionReader {
  public static void main(String[] args) throws Exception {
    URL url = new URL("https://www.youporn.com/");
    URLConnection http = url.openConnection();

    InputStream httpInputStream = null;
    InputStreamReader inputStreamReader = null;

    try {
      httpInputStream = http.getInputStream();

      http = null; // I set it to null to do the test
      System.out.println("http " + (http == null ? "==" : "!=") + " null");

      // ...and httpInputStream??
      System.out.println("httpInputStream " + (httpInputStream == null ? "==" : "!=") + " null");

      inputStreamReader = new InputStreamReader(httpInputStream);
      StringBuilder stringBuilder = new StringBuilder();
      char[] charBuffer = new char[2048];
      int num;

      while ((num = inputStreamReader.read(charBuffer)) != -1) {
        stringBuilder.append(charBuffer, 0, num);
      }

      System.out.println(stringBuilder.toString());
    } catch (Exception ex) {
      System.out.println("Exception: " + ex);
    } finally {
      if (http != null) {
        System.out.println("http != null");

        if (httpInputStream != null) {
          System.out.println("httpInputStream != null");
          httpInputStream.close();
        }
      }
    }
  }
}

Let's run the program and see what is printed:

C:\Projects\UnreleasedStream> javac URLConnectionReader.java
C:\Projects\UnreleasedStream> java URLConnectionReader
http == null
httpInputStream != null
[...page body...]

As we can see, even if http is null, httpInputStream has NOT become automagically null, and the body of the page loads without problems and no finally block messages are printed.

Of course, the exact finally block should look like this:

finally {
  if (httpInputStream != null) {
    try {
      httpInputStream.close();
    } catch (IOException ioex) {
      System.out.println("IOException: " + ioex);
    }
  }
}

It's not necessary to close the URLConnection because closing the stream connected to it should free up the network resources associated with that instance.

But when we close a stream we have to catch a potential IOException which could be thrown for several reasons: if we have opened a file and the operating system changes the metadata of that file during the closing (it practically never happens, but I always prevent any apocalyptic scenario), or in the case of reading an input stream from a connection and at the moment of closure the network is down (most likely), or other various and possible ones.

However, if an already closed stream is closed, nothing is reported. At least that.

Extra

In this article I wanted to point out that sometimes we can get a little confused about how to manage things when we develop, giving the example of a fact that really happened.

Writing the small test program, however, I added another variant to take into consideration, that is the use of the InputStreamReader which, as we have seen, had not been closed in the previous code because that was not the demonstration purpose at that time.

To be fair, the exact final code should be this (I renamed http and httpInputStream because they were misleading):

import java.net.*;
import java.io.*;

public class URLConnectionReader {
  public static void main(String[] args) throws Exception {
    URL url = new URL("https://www.youporn.com/");
    URLConnection urlConnection = url.openConnection();

    InputStream inputStream = null;
    InputStreamReader inputStreamReader = null;

    try {
      inputStream = urlConnection.getInputStream();
      inputStreamReader = new InputStreamReader(inputStream);

      StringBuilder stringBuilder = new StringBuilder();
      char[] charBuffer = new char[2048];
      int num;

      while ((num = inputStreamReader.read(charBuffer)) != -1) {
        stringBuilder.append(charBuffer, 0, num);
      }

      System.out.println(stringBuilder.toString());
    } catch (Exception ex) {
      System.out.println("Exception: " + ex);
    } finally {
      if (inputStreamReader != null) {
        try {
          inputStreamReader.close();
        } catch (IOException ioex) {
          System.out.println("IOException: " + ioex);
        }
      }
    }
  }
}

Closing an InputStreamReader releases all associated resources, so it is not necessary to close the InputStream.

If there are any doubts about this, here is another test program:

import java.net.*;
import java.io.*;

public class ReaderCloser {
  public static void main(String[] args) throws Exception {
    InputStream inputStream = null;
    Reader reader = null;

    try {
      inputStream = new FileInputStream("c:\\youpornlist.txt");
      reader = new InputStreamReader(inputStream);
      reader.close();
      inputStream.read(); // Crash! The reader has already closed everything!
    } catch (Exception ex) {
      System.out.println("Exception: " + ex);
    }
  }
}

This program crashes with a nice Exception: java.io.IOException: Stream Closed because you are trying to close inputStream when it has already been closed by the reader.

PS: I used that filename because I'm sure we all have it on the computer.

Test #02

I'd say the above test might be enough, but you might be wondering:

What if instead of setting http to null, which maybe still keeps a connection hanging somewhere, I close the connection??

In fact it's a good question, good.

The problem is that URLConnection has no methods to close or disconnect the connection. But we can try with the HttpURLConnection which instead has disconnect().

import java.net.*;
import java.io.*;

public class HttpURLConnectionReader {
  public static void main(String[] args) throws Exception {
    URL url = new URL("https://www.youporn.com/");
    HttpURLConnection http = (HttpURLConnection)url.openConnection();

    InputStream inputStream = null;
    InputStreamReader inputStreamReader = null;

    try {
      inputStream = http.getInputStream();

      http.disconnect(); // Disconnect everything
      System.out.println("http " + (http == null ? "==" : "!=") + " null");

      // ...and inputStream??
      System.out.println("inputStream " + (inputStream == null ? "==" : "!=") + " null");

      inputStreamReader = new InputStreamReader(inputStream);
      StringBuilder stringBuilder = new StringBuilder();
      char[] charBuffer = new char[2048];
      int num;

      while ((num = inputStreamReader.read(charBuffer)) != -1) {
        stringBuilder.append(charBuffer, 0, num);
      }

      System.out.println(stringBuilder.toString());
    } catch (Exception ex) {
      System.out.println("Exception: " + ex);
    } finally {
      if (http != null) {
        System.out.println("http != null");

        if (inputStream != null) {
          System.out.println("inputStream != null");
          inputStream.close();
        }
      }
    }
  }
}

Let's run the program and let's get the output:

C:\Projects\UnreleasedStream> javac HttpURLConnectionReader.java
C:\Projects\UnreleasedStream> java HttpURLConnectionReader
http != null
inputStream != null
Exception: java.io.IOExpection: stream is closed
http != null
inputStream != null

In this case the program crashes, because we closed the connection and then the stream opened by inputStream is no longer valid. Note that they are all different from null anyway.

The finally block in the case of using an HttpURLConnection, however, is the following:

finally {
  if (inputStream != null) {
    try {
      inputStream.close();
    } catch (IOException ioex) {
      System.out.println("IOException: " + ioex);
    }
  }

  if (http != null) {
    http.disconnect();
  }
}

This is because each instance of HttpURLConnection is used to make a single request but the underlying network connection to the HTTP server can be transparently shared by other instances.

With the close() method of the InputStream and OutputStream used with an HttpURLConnection, the network resources associated with this instance are freed but there is no effect on any shared connection. Using the HttpURLConnection disconnect() instead, we close the socket if there are no other active connections at that time.

Garbage Collector

The release of resources should always be managed. I know that with some languages we have the Garbage Collector support, but are you really sure? Do you know it so well? Have you ever gone to dinner together? Are you sure it does it right and when needed? Would you ever entrust your dog to it?

If you answered "yes" to all the questions (or at least the dog's question), I'm sorry to tell you that Java's Garbage Collector frees memory that is no longer referenced, but it cannot free resources such as open file descriptors or connections to the database.

This is because basically he takes care of the resources inside the heap area of the JVM, while the resources assigned by the operating system such as files or connection openings are external to this management. So you have to be careful.

Conclusion

Using threads and streams is always quite delicate. Many developers fall into the intrigue of patterns and best practices related to these two topics. I myself, who am a very good, beautiful, but above all modest developer, sometimes lose absolute control of the situation and directly format the server.

But in this specific case, I would say that if a class has the close() method there is a reason right? Use it! Or at least ask yourself two questions about why it exists.

Anyway, when in doubt, throw everything in the finally block, don't leave anything to chance, don't trust the Garbage Collector and don't accept sweets from sysadmins.

Stay safe, stay close();