Resource leak warning when using Scanner

Question

I'm watching some java tutorials and wondering about the resource leak warning when using Scanner.

I know I can close it, but the person in the video is not getting a warning even though he is using the exact same code, why is that?

    Scanner input= new Scanner(System.in);
    
    System.out.print("Enter a line of text: ");
    String line = input.nextLine();
    
    System.out.println("You entered: " + line);
    
    //input.close();

Answer 1

The actual problem

It's.. complicated. The general rule is that whomever makes the resource must also safely close it, and because your code makes a Scanner, the IDE is telling you: Hey, you should close that.

The problem is, closing that scanner is wrong here: The scanner wraps around System.in, which is NOT a resource you made, but scanner.close() would close the underlying stream (System.in itself), and you don't want that: It's not your responsibility and in fact actively harms things; now you can never read from sysin again.

The problem is, the IDE can't really know this. The underlying problem is that System.in is extremely badly designed API in many many ways, but [A] it's 30 years old, back then it was a lot harder to know that; we know it now because of hindsight, and [B] oracle hasn't gotten around to making a second version of the sysin/out/err API yet, and it's not high on the agenda.

This leaves IDEs in trouble: It's relatively easy to set up some patterns and rules for using resources such that you never have any problems with this 'filters you created that wrap around resources you did not create', but you can't use them with sysin/err/out without writing a little framework, and that's a bit much to ask for newbies. It's also not a good idea presumably to tell those taking their first steps in java coding to first go download some third party library that cleans up sysin/out/err interaction a bit.

Thus, we're in limbo. IDEs should NOT warn about this, but it's hard for them to detect that this is an exotic sccenario where you have a resource you made that you nevertheless don't need to close, and in fact, should not close.

You can turn off the setting for 'unclosed resources', which no doubt the video tutorial did, but it is a useful warning. Just.. hampered by this silly old API that makes no sense anymore.

Some in-depth explanation

There are resources. These are things that implement AutoClosable, and there are very many. Let's focus on those that represent I/O things: The top level types in the hierarchy are Writer, Reader, InputStream, and OutputStream (let's call them all WRIOs). They're all AutoCloseable, and most IDEs (incorrectly?) all complain about unclosed resources for these. However, that's oversimplifying things.

You can split the world of all WRIOs into:

• Actual resources (they directly represent an underlying OS-based concept that results to starvation of some resource, a.k.a. 'a leak', if you do not close them). new FileInputStream, socket.getInputStream - etc, these represent actual resources
• Dummy resources - they act like a resource but don't actually represent a resource that you can starve out that isn't already fixed by the garbage collector. new ByteArrayInputStream, turning StringBuilders into Readers, etc.
• filters - these wrap around a resource and modify it 'in transit'. Such filters do not themselves capture any starvable resource. If you close() them, they also invoke close on the thing they wrap. Scanner is a filter.

The rules on closing them boil down to:

• Actual resources - must be closed safely by whomever made them. IDE warnings are warranted if you fail to do this. Note that you did not make System.in, so doesn't apply there.
• Dummy resources - you can close them, but you don't have to. If the IDE warns on them, toss a try-with-resources around it, annoying but not too hard to work around.
• Filters - tricky.

The problem with filters

If it's a filter provided to you with the intent that you close it:

BufferedReader br = Files.newBufferedReader(somePath);

then failure to close br is a resource leak; IDE warnings are warranted.

If it's a filter you made, wrapping around a WRIO you also made:

InputStream raw = socket.getInputStream();
BufferedReader br = new BufferedReader(new InputStreamReader(raw, StandardCharsets.UTF_8));

(This is 1 real resource, wrapped by a filter WRIO (InputStreamReader), and then that filter wrapped by another filter WRIO): Then the resource leak is all about raw, and if you fail to safely close br, that's no resource leak. It might be a bug (if you close raw without closing/flushing br first, a bunch of bytes in the buffer won't have been written out), but not a resource leak. An IDE warning about failure to close br is wrong, but not too harmful, as you can just toss try-with-resources around it, and this in passing also guarantees that 'bug due to failure to flush out the buffering filter WRIO' cannot happen anymore.

Then, there is the problem case:

Making a filter WRIO that wraps around a resource you did not make and do not have the responsibility to close: You should actively NOT be closing these filter WRIOs, as that will end up closing the underlying resource and you did not want that.

Here an IDE warning is actively bad and annoying, but it is very hard for an IDE to realize this.

The design solution

Normally, you fix this by never getting in that scenario. For example, System.in should have better API; this API would look like:

try (Scanner s = System.newStandardIn()) {
   // use scanner here
}

and have the property that closing s does not close System.in itself (it would do mostly nothing; set a boolean flag to throw exceptions if any further read calls are done, or possibly even do literally nothing). Now the IDE warning is at best overzealous, but heeding its advice and safely closing your scanner is now no longer actively introducing bugs in your code.

Unfortunately, that nice API doesn't exist (yet?). Thus we're stuck with this annoying scenario where a useful IDE warning system actively misleads you because of bad API design. If you really want to, you could write it:

public static Scanner newStandardIn() {
    Scanner s = new Scanner(System.in) {
        @Override public void close() {}
    };
    // hey, while we're here, lets fix
    // another annoying wart!

    s.useDelimiter("\r?\n");
    return s;
}

Now you can heed those warnings by following its advice:

public static void main(String[] args) {
    String name;
    int age;

    try (Scanner s = newStandardIn()) {
        System.out.print("What is your name: ");
        // use next() to read entire lines -
        // that useDelimiter fix made this possible
        name = s.next();
        System.out.print("What is your age: ");
        age = s.nextInt();
    }

    // use name and age here
}

no IDE warning, and no bugs.