How do I fix a memory leak in java

Question

I'm currently trying to write a program that get a stringified json object from steam and uses the object to determine if I can buy an item on the steam market or not.

It works, however I seem to be getting massive memory leaks and I have no idea how to solve this problem as I'm a beginner programmer. Here is the code:

import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.net.URLConnection;
import java.util.Scanner;


public class SteamMarketAlert {

    @SuppressWarnings("unused")
    private JAlertWindow alert;
    private URL jsonUrl;
    private float walletValue;
    private boolean itemBuyable;

    public SteamMarketAlert(URL itemUrl, float walletValue)
    {
        this.itemBuyable = false;
        this.jsonUrl = getJSONurl(itemUrl);
        this.walletValue = walletValue;
    }

    private URL getJSONurl(URL itemUrl)
    {
        String jsonString = itemUrl.toString();

        String firstPart = jsonString.substring(0, jsonString.indexOf("market/") + "market/".length());

        String appid = jsonString.split("/")[5];
        String marketHashName = jsonString.split("/")[6];

        String secondPart = "priceoverview/?currency=2&appid=" + appid + "&market_hash_name=" + marketHashName;

        try {
            return new URL(firstPart + secondPart);
        } catch (MalformedURLException e) {
            System.err.println("Failed to create json url");
            return null;
        }

    }

    public void checkMarket()
    {
        Thread thread = new Thread(){
            @Override
            public void run(){
                try {
                    while(!itemBuyable)
                    {
                        sleep(5000);                        
                        if(isBuyable(getPagehtml()))
                            itemBuyable = true;
                    }
                    alert = new JAlertWindow();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }   
        };

        thread.start();


    }

    private boolean isBuyable(String pagehtml)
    {
        int firstIndex = pagehtml.indexOf(";") +1;


        float marketValue = Float.parseFloat(pagehtml.substring(firstIndex, firstIndex + pagehtml.substring(firstIndex, pagehtml.length()).indexOf("\"")));

        return (marketValue <= walletValue)? true:false;
    }

    private String getPagehtml(){

        try(Scanner scanner = new Scanner(jsonUrl.openConnection().getInputStream())) {

            scanner.useDelimiter("\\Z");
            return scanner.next();
        } catch (IOException e) {               
            e.printStackTrace();
            return null;
        }

    }

    public static void main(String[] args)
    {
        try {
            float walletValue =  82.64f;
            URL itemUrl = new     URL("http://steamcommunity.com/market/listings/730/StatTrak%E2%84%A2%20P90%20%7C%20Asiimov%20%28Factory%20New%29");
            SteamMarketAlert sma = new SteamMarketAlert(itemUrl,walletValue);
            sma.checkMarket();


        } catch (MalformedURLException e) {

            e.printStackTrace();
        }

    }
}

I've narrowed the problem down to the checkMarket() method. However, I can't seem to figure out what is going on. Can you please point out how I can fix this (and possibly point out all the flaws in my code), Note the JAlertWindow object just displays a JFrame with "CAN BUY" on it - nothing special.

edit: I updated the code since posting and users informed me that try-with-resource blocks existed. Thank you to everyone who has helped me understand how java garbage collection works. :)!

For one thing, there is no need to have `html` declared outside the `while` loop in the Thread. And actually, you don't need the `StringBuilder` at all, just return the value of `scanner.next()` from `getPagehtml`. Not that I'd say that will have much impact on a memory leak, it's just a bit clumsy. — Andy Turner, Jul 10 '15 at 22:43
Please describe what exactly you mean by "*memory leak*" and how you can prove its existence. — PM 77-1, Jul 10 '15 at 22:48
Well when I export it to a .jar file and launch the jar, while its running in the task manager the memory value goes up over time, so to begin with it starts at around 12,000 K and goes up to around 30,000 K in about 10 minutes... and keeps on rising :P. That's what I mean by memory leak. — James, Jul 10 '15 at 22:57
@AndyTurner How can I close the scanner if I'm returning the value while the scanner is still open? I mean "return scanner.next():" surely breaks me out of the getPagehtml() method without closing the scanner right? I was under the impression that I had to close the scanner. — James, Jul 10 '15 at 23:14
if you are not getting `OutOfMemoryException` you do not have a problem. — , Jul 11 '15 at 00:50
@James you should use a try-with-resources block. Alternatively, you can declare a variable and assign the value, then close the scanner, then return it. However, the latter method is less robust. — Andy Turner, Jul 11 '15 at 07:03
@AndyTurner I had no idea that try-with-resource blocks existed, I looked them up and I've updated my code (in stack overflow as well as my own copy) thank you for your help! :) — James, Jul 11 '15 at 18:20

score 1 · Accepted Answer · answered Jul 10 '15 at 23:03

1

It's provbably not a memory leak in Java until you get an OutOfMemoryException or you see constant garbage collection.

Using 18MB in 10 minutes doesn't seem like a memory leak, that's just how Java works. If you really want to make sure, you can turn on verbose GC and see how often it's collecting, but I don't think you have a real issue yet.

answered Jul 10 '15 at 23:03

AngerClown

6,149
1
25
28

Thank you, I just saw the memory keep on climbing and thought it was an issue - especially considering I'm going to be running this for quite some time. – James Jul 10 '15 at 23:07

score 0 · Answer 2 · answered Jul 11 '15 at 00:35

You're fetching html pages and because of that some objects (String etc) have to be allocated in memory on every get. Later on (once no longer used) these objects will be deleted and memory marked as free by GC.

Sorry for quite a simplistic explanation. if you want to know more just read about about java GC, heap structure etc.

score -4 · Answer 3 · answered Jul 10 '15 at 22:45

-4

Why not run a Java profiler? I recommend using YourKit. I use it a lot for finding the cause of any memory leaks I have on my servers. It is great for connecting to remove applications too; you should play around with it.

Here's a link to their own video on how to find a memory leak; Link

If you need more help you should have a quick read through their docs

answered Jul 10 '15 at 22:45

md_5

27
6

3

This is really a comment, not an answer. With a bit more rep, [you will be able to post comments](http://stackoverflow.com/privileges/comment). – PM 77-1 Jul 10 '15 at 22:47
Yeah I never downvoted anything you said, I will keep this comment in mind in future if I have trouble finding memory leaks :). Thank you for your comment. (The main issue I was asking about wasn't finding the leak it was more about how I can modify the code so that there is no leak) – James Jul 10 '15 at 23:03

How do I fix a memory leak in java

3 Answers3