Public Group

Is this memory leakage?

This topic is 1440 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

So I am wondering if what I did here is something that is bad or if the java garbage collector will take care of it on its own. I have two different methods below and I have taken out all irrelevant code to simplify things. I create a scanner in each method. Usually I would close the scanners to get rid of the memory leak warning, however, when I close the first scanner, I was unable to use the second. I found a solution that said just don't close the scanners and it works fine, but I am wondering if this is dangerous to do. Thanks in advance!

static String menu(){

static void info(){

Scanner infoscanner = new Scanner(System.in);

infoscanner.next();

}//end of info method



Share on other sites

System.in is really standard input, and your program only gets one of these. Once you close it, that's it - you can't get any further input from the user from standard input ("closing" a scanner closes the underlying stream). So, you generally should not close it if you are going to be using it again later, and it is not a memory leak, as it is closed when you shut down your program anyway. This is one of the exceptions to the rule of "close what you open", because System.in actually belongs to your process.

Share on other sites
Another approach would be to create the scanner in main(), and pass it to these methods. Alternatively, the scanner could be a field on the class where these methods are, and they could be made non-static.

Share on other sites

Best practice would be to re-use the same scanner instance. If the menu() and info() method must remain static then you can do something like this:

private static final Scanner scanner = new Scanner(System.in);

//You should specify either if the method is public or private.

//Do some stuff...

}

//You should specify either if the method is public or private.
private static void info(){
String info = scanner.next();

//Do some stuff...
}

Edited by PsychotikRabbit

Share on other sites

I believe this usage of the Scanner class is a resource leak.

You should be calling close() on the scanner reference after each use (unless ideally your architecture is such that you can use a single reference and pass it around). This should only clean up the scanner instance but leave stdin untouched for future use.

If you have an Exception thrown (or you return) between where you have instantiated scanner and when you call close() then you have a leak. So you will probably want to add a bunch of try catch finallys around your code (inside the finally you have the close() call).

C++ deals with this (and memory leaks) in a much more elegant fashion with RAII, C# has the "using" pattern (which unfortunately only works in function scope) but with Java you unfortunately need to fall back to the try catch stuff.

Unless you are happy for your code to not be exception safe (which is likely to not really be a problem for many games), in which case just remember to call close().

Edited by Karsten_

Share on other sites

Yes you are right you need to call close.

Another approach:

public class InputManager {
private final Scanner scanner;

public InputManager() {
scanner = new Scanner(System.in);
}

//This should be called when you don't want to read input anymore.
public void shutdown() {
scanner.close();
}

//Do some stuff...

}

public void info(){
String info = scanner.next();

//Do some stuff...
}
}


You could then use your class like this:

public static void main(String[] args) {
InputManager inputManager = new InputManager();

//Do whatever you want with the user input.
System.out.println("Option: " + option);

//Before exiting the application you free resources
inputManager.shutdown();
}

Edited by PsychotikRabbit

• 45
• 12
• 10
• 10
• 9
• Forum Statistics

• Total Topics
631373
• Total Posts
2999630
×