Advertisement

Is this memory leakage?

Started by July 06, 2014 11:26 PM
4 comments, last by PsychotikRabbit 10 years, 6 months ago

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(){
 
String menuoption;
Scanner menuscanner = new Scanner(System.in);
 
menuoption = menuscanner.next();
 
return menuoption;
 
}// end of menu method
 
 
static void info(){
 
Scanner infoscanner = new Scanner(System.in);
 
infoscanner.next();
 
}//end of info method
 

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.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

Advertisement
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.

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. 
private static String menu(){
   String menuoption = scanner.next();
 
   //Do some stuff...

   return menuoption;
}
 


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

   //Do some stuff... 
}

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().

http://tinyurl.com/shewonyay - Thanks so much for those who voted on my GF's Competition Cosplay Entry for Cosplayzine. She won! I owe you all beers :)

Mutiny - Open-source C++ Unity re-implementation.
Defile of Eden 2 - FreeBSD and OpenBSD binaries of our latest game.

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();
   }

   public String menu(){
      String menuoption = scanner.next();
 
      //Do some stuff...

      return menuoption;
   }
 
   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.
   String option = inputManager.menu();
   System.out.println("Option: " + option);

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

This topic is closed to new replies.

Advertisement