jackson-databind: Stream not closed if IOException thrown while creating parser

@cowtowncoder Thanks for the quick fix. I can confirm that the read-case via File is working correctly.

@josh-byster I did a quick look at the initial issue and it seems to be an upstream issue in Java. Observed behavior with Java 17:

• URL#openStream creates a URLConnection depending on the URL that cannot be closed (no close method).
• In this case, a sun.net.www.protocol.http.HttpURLConnection instance is created.
• Afterwards URLConnection is used to create the input stream via getInputStream. HttpURLConnection#getInputStream is mostly a proxy for getInputStream0.
• getInputStream0 creates the input stream and assignes it to itself.
• A FileNotFoundException exception is thrown due to HTTP 404 but the input stream isn’t closed. See note *1.

Actually, there is a way to close the internal input stream by calling disconnect of HttpURLConnection. With a bit of magic, all sockets are successfully closed (see example below). Unfortunately, we cannot use this workaround because it’s an internal Sun class.

*1: AFAIK, the input stream is internally never closed but maybe I’m missing something. The code is mostly 15 years old and not well structured 😄 . The exception in HttpURLConnection seems to prevent the GC from freeing the HttpURLConnection instance that was created in URL#openStream (maybe due to the input stream that is never closed). HttpURLConnection most likely own the socket too.

I just stepped though a few files but you could continue to locate the core issue if you like, or create an upstream issue. If you have this issue at work, my best bet is to report it to your “Java vendor” for prio support. Let me know, otherwise I’ll eventually report the potential bug.

Tested with:

openjdk version "17.0.3" 2022-04-19
OpenJDK Runtime Environment 21.9 (build 17.0.3+7)
OpenJDK 64-Bit Server VM 21.9 (build 17.0.3+7, mixed mode, sharing)

Example using nodejs server from above:

{
  URLConnection connection = new URL("http://localhost:3001/").openConnection();
  // try (InputStream stream = new URL("http://localhost:3001/").openStream())
  try (InputStream stream = connection.getInputStream()) {
      System.out.println("Ok");
  } catch (IOException ex) {
      ex.printStackTrace();
  }

  // # Manually, closing the connection:
  // // Add compiler flag: --add-exports java.base/sun.net.www.protocol.http=ALL-UNNAMED
  // connection.getClass().getMethod("disconnect").invoke(connection);
}

Thread.sleep(Integer.MAX_VALUE);

It might make sense to remove the description and just add a mention of #3508 as possibly related.

I do not understand what you mean by this. Do you want to merge the issues?

I’d still like to know what is happening on reader side since I do not yet see a plausible scenario.

The managed stream by Jackson is only closed after successfully initialization when _readMapAndClose is executed. In the error case, the exception occurs during initialization and the stream is leaked:

1. JsonFactory#createParser(File f) creates an input stream from the given file.
2. ByteSourceJsonBootstrapper#constructParser in jackson.core raise an IOException due to a failure in detectEncoding (see stack trace above).
3. The parser, e.g. UTF8StreamJsonParser which should own the input stream, is never created and we never execute _readMapAndClose.
4. The input stream from JsonFactory#createParser(File f) is leaked because neither the initial method nor UTF8StreamJsonParser closes the stream.

The same will most likely apply to URL as well.

A simple solution would be to create the stream with try-with-resources and call readValue(InputStream src, ...). Otherwise, catching the exception from _createParser in JsonFactory#createParser and closing the stream in an error case would work too. I’m a little bit lost about the ownership of the input stream in the latter case because ByteSourceJsonBootstrapper doesn’t own the stream at this moment.