Jump to content

  • Log In with Google      Sign In   
  • Create Account

Is this memory leakage?


Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.


  • You cannot reply to this topic
5 replies to this topic

#1 Rick281   Members   -  Reputation: 146

Like
0Likes
Like

Posted 06 July 2014 - 05:26 PM

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
 


#2 Bacterius   Crossbones+   -  Reputation: 12885

Like
3Likes
Like

Posted 06 July 2014 - 05:39 PM

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


#3 rip-off   Moderators   -  Reputation: 10536

Like
0Likes
Like

Posted 07 July 2014 - 01:31 AM

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.

#4 PsychotikRabbit   Members   -  Reputation: 175

Like
0Likes
Like

Posted 10 July 2014 - 06:46 AM

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

Edited by PsychotikRabbit, 10 July 2014 - 06:52 AM.


#5 Karsten_   Members   -  Reputation: 2091

Like
0Likes
Like

Posted 10 July 2014 - 07:37 AM

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_, 10 July 2014 - 07:38 AM.

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


#6 PsychotikRabbit   Members   -  Reputation: 175

Like
1Likes
Like

Posted 10 July 2014 - 07:48 AM

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

Edited by PsychotikRabbit, 10 July 2014 - 07:52 AM.





Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.




PARTNERS