How can I fix my memory leak?

Question

I made this small script to ping google.pt and do something with the ping result. The problem is that if I let the script run for a while, it uses more and more RAM.

I can't seem to find the mistake, can you help me?

public static void main(String[] args) {
    String ip = "google.pt -t -4";
    int pingRetrieved = 0;

    //window that displays the ping
    s = new Square();


    String pingCmd = "ping " + ip;
    try {


        Runtime r = Runtime.getRuntime();

        Process p = r.exec(pingCmd);

        BufferedReader in = new BufferedReader(new
        InputStreamReader(p.getInputStream()));
        String inputLine;
        while ((inputLine = in.readLine()) != null) {
            //System.out.println(inputLine);
            pingRetrieved = getPingValueFromPingResult(inputLine);
            takeActionFromPingValue(pingRetrieved);
        }
        in.close();

    } catch (IOException e) {
        System.out.println(e);
    }


}

EDIT: the getPingValue method:

private static int getPingValueFromPingResult(String inputLine) {
    String[] splitString;
    String timeParameter;
    if (inputLine.contains("Reply")) {
         splitString = inputLine.split(" ");
         timeParameter = splitString[4];
         timeParameter = (timeParameter.split("="))[1];
         timeParameter = timeParameter.replace("ms", "");
         return Integer.parseInt(timeParameter);
    }
    return 0;
}

and the take actionFromPingValueMethod just calls this on a jframe that I created:

public void printNumber(int ping){

    this.getContentPane().removeAll();
    JLabel jl = new JLabel();
    Font f;
    if(ping == 0){
        this.setLocation((int) (Toolkit.getDefaultToolkit().getScreenSize().getWidth()-200), 0);
        jl = new JLabel("Connection Error");
        f = new Font("Fixedsys", Font.PLAIN, 25);
    }else{
        this.setLocation((int) (Toolkit.getDefaultToolkit().getScreenSize().getWidth()-100), 0);
        jl = new JLabel(ping+"ms");
        f = new Font("Fixedsys", Font.PLAIN, 25);
    }
    jl.setFont(f);
    jl.setForeground(Color.MAGENTA);
    this.getContentPane().add(jl, java.awt.BorderLayout.NORTH);
    this.pack();

}

What do the other methods do `getPingValueFromPingResult` and `takeActionFromPingValue`. — M. Deinum, Oct 11 '16 at 12:53
The part of the code that your posted here has no apparent problems. It's likely in the missing methods. — f1sh, Oct 11 '16 at 12:59
You says it use more and more RAM. Can you quantify? Have you try to set the JVM max memory ? — jhamon, Oct 11 '16 at 13:05
It is normal for a Java application to slowly accumulate more and more RAM use until the maximum allowed amount is allocated. The JVM doesn't return memory to the OS, because it does its own memory management within the memory allocated. See "garbage collection". — JimmyB, Oct 11 '16 at 13:12
When I start running the script, it uses about 30 megabytes. After 2 or 3 hours of it running it uses 60-80 megabytes — Miguel Freitas, Oct 11 '16 at 13:17
@MiguelFreitas That sounds perfectly normal. See also http://stackoverflow.com/questions/5374455/what-does-java-option-xmx-stand-for. — JimmyB, Oct 11 '16 at 13:19
@MiguelFreitas My best bet is the printNumber function. It is the only part of the code where i dont see where the variables end (Due to UI handling). Try to create the JLabel as a Static Variable and set it up once, then update the JLabel with the setText Function — Flying Dutch Boy, Oct 11 '16 at 13:20
@FlyingDutchBoy Thank you for the suggestion. I will try that. — Miguel Freitas, Oct 11 '16 at 13:21
Maybe also have a look at http://stackoverflow.com/questions/4667483/how-is-the-default-java-heap-size-determined. — JimmyB, Oct 11 '16 at 13:24
@FlyingDutchBoy It should be a field of the class but not *static*. — JimmyB, Oct 11 '16 at 13:26
@JimmyB Sorry i scoped out. The Jframe should be a static reference (should not be created for every call) and then it should have a field JLabel and a constructor that does the font, color and location stuff. That should help with a double JLabel constrution in the print function — Flying Dutch Boy, Oct 11 '16 at 13:30
@JimmyB Ok, I changed it. The Jframe was never being created every call, but the JLabels were. Now I put the JLabel as a class field, and I'll see if it helps. Thanks a lot for the help! — Miguel Freitas, Oct 11 '16 at 13:33
Maybe also note that Swing may be allocating and discarding objects in the background which you are not aware of. And that `String`s are objects too, although you don't allocate them with `new`. — JimmyB, Oct 11 '16 at 14:57

score 0 · Answer 1 · answered Apr 18 '17 at 12:26

0

the ping command normally runs forever and thus your condition

while ((inputLine = in.readLine()) != null)

only will be false when your timeout (specified with -t) triggers. You should limit the number of ping request by using the -c count parameter.

answered Apr 18 '17 at 12:26

P.J.Meisch

18,013
6
50
66

How can I fix my memory leak?

1 Answers1