0

I have a question about memory leak.I have two classes.

The first one is:

public class Utility {
private static Utility instance = null;
private UpdateListener listener;

//Make it a Singleton class
private Utility(){}
public static Utility getInstance() {
    if (instance == null)
        instance = new Utility();
    return instance;
}

public void setListener(UpdateListener listener) {
    this.listener = listener;
}

//Long running background thread
public void startNewTread() {
    new Thread (new Runnable() {
        @Override
        public void run() {
            try {
                Thread.sleep(1000 * 10);
                if (listener != null)
                    listener.onUpdate();
            } catch (InterruptedException e) {
                Log.d("Utility", e.getMessage());
            }
        }
    }).start();
}

//Listener interface
public interface UpdateListener {
    public void onUpdate();
}
}

Thesecond class is:

public class ListenerLeak extends AppCompatActivity {

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    //Setting the listener
    Utility.getInstance().setListener(new Utility.UpdateListener() {
        @Override
        public void onUpdate() {
            Log.d("ListenerLeak", "Something is updated!");
        }
    });

    //Starting a background thread
    Utility.getInstance().startNewTread();
}

@Override
protected void onDestroy() {
    super.onDestroy();
}
}

in this activity.May new Utility.UpdateListener create a memory leak? when the activity destoroyed , only Updatelistener can be alive.does activity can be alive?

Phantômaxx
  • 37,901
  • 21
  • 84
  • 115
ahmetvefa53
  • 581
  • 6
  • 20

2 Answers2

0

Create an inner class inside a Utility class like below. Then move the thread to that class.

 public void startNewTread() {
    new MyThread().start();
    }

 private static class MyThread extends Thread {
    @Override
    public void run() {
       try {
            Thread.sleep(1000 * 10);
            if (listener != null)
                listener.onUpdate();
        } catch (InterruptedException e) {
            Log.d("Utility", e.getMessage());
        }
    }
  }

Reason: After each configuration change, the Android system creates a new Activity and leaves the old one behind to be garbage collected. However, the thread holds an implicit reference to the old Activity and prevents it from ever being reclaimed. As a result, each new Activity is leaked and all resources associated with them are never able to be reclaimed. https://www.androiddesignpatterns.com/2013/04/activitys-threads-memory-leaks.html will help to understand it.

PushpikaWan
  • 2,437
  • 3
  • 14
  • 23
  • someone told me leak reason is this code :Utility.getInstance().setListener(new Utility.UpdateListener() { @Override public void onUpdate() { Log.d("ListenerLeak", "Something is updated!"); } }); – ahmetvefa53 Oct 30 '18 at 16:25
  • also here is thread is not inner class of activity .but creating a anonymous UpdateListener – ahmetvefa53 Oct 30 '18 at 16:26
  • did u try both solutions? what are the results? – PushpikaWan Oct 30 '18 at 16:26
  • no I did'nt try.How can I see result ? anroid profiler? – ahmetvefa53 Oct 30 '18 at 16:28
  • u can use profiler in android studio. https://stackoverflow.com/questions/47501739/how-to-treat-memory-leaks-using-the-new-androidprofiler this will help for u. – PushpikaWan Oct 30 '18 at 16:31
  • this also help for u https://gist.github.com/lopspower/c65f28ee504763bd0b4a – PushpikaWan Oct 30 '18 at 16:32
  • I am trying now but I didnt understand . why reasons is thread.because thread is not in activity. – ahmetvefa53 Oct 30 '18 at 16:35
  • as I mentioned in answer "Android system creates a new Activity and leaves the old one behind to be garbage collected. However, the thread holds an implicit reference to the old Activity and prevents it from ever being reclaimed". I think u know what is memory leak means. – PushpikaWan Oct 30 '18 at 16:38
  • I know but here thread is not related to activity.listener is related to activity.I mean if activity destroys , listener can be null? – ahmetvefa53 Oct 30 '18 at 16:46
  • İn this link , I asked a question and someone did answer it . But ı didnt understand why there is a memory leak. https://stackoverflow.com/questions/53068236/memory-leak-with-interface-referance/53068506?noredirect=1#comment93036403_53068506 Can you explain ? – ahmetvefa53 Oct 30 '18 at 18:34
  • Every time you start a worker thread from an activity, you are responsible of managing the worker thread yourself. Because the worker thread can live longer than the Activity, you should stop the worker thread properly when the Activity is destroyed. If you forget to do so, you are risking leaking the worker thread itself. Example is here https://github.com/frank-tan/SinsOfMemoryLeaks/blob/LEAK/app/src/main/java/com/franktan/memoryleakexamples/vialongrunningtask/LeakThreadsActivity.java – PushpikaWan Oct 31 '18 at 00:59
0

This is probably a bit late, and others have had their input as well, but I'd like to have my shot as well :). Memory leak simply means that GC is not able to release a memory used by an instance of an object because it can't be sure that whether it is being used or not.

And in your case, simply put: The Utility class is defined as Singletone, It has a static instance of itself in the class. So It will be there as long as the application is alive. When you set a listener from the activity using setListener() function you are passing an instance created in the activity to it that has a limited lifecycle and is bound to activity's lifecycle. So one can say that the static Utility class can outlive the listener instance passed to utility and leak the activity. So no matter if you're using thread or not, This leaks the activity instance because it can outlive the listener instance which has an implicit reference to parent activity class.

How to prevent leaks here? I think using a WeakReference for the listener is a good starting point, Also making sure to release or remove the listener as soon as the onDestroy() method of activity is called. but as documentations state, there's no guaranty that onDestroy() is always called. So in my opinion going with something like onPause() or onStop() is a better idea.

Ruben Helsloot
  • 12,582
  • 6
  • 26
  • 49
Rez
  • 4,501
  • 1
  • 30
  • 27