1

I am building an android app that fetches data from a hardcoded url.

Because the fetching can take a while, I first wrote an interface to support callbacks

public interface MyCallBack {
   void data_received(String data);
}

The in the main activity, I implemented MyCallBack

public class MainActivity extends AppCompatActivity implments MyCallBack {

  @Override
  public void data_received(String data) {
    Log.d("got data", data);
    // do something useful with data...
  }

  public void btn_getData_clicked(View view) {
       // a button is clicked, and fetching is started:
       BackgroundWorker bw = new BackgroundWorker(this);
       bw.execute("https://randomuser.me/api/");

      // after this point, bw does not leave the heap, 
     //  because BackgroundWorker keeps a reference to (this)
  }
}

I want my code to make a separate fetch whenever the button is pressed. But I also want the consumed/done/already called back instances to go away, and leave the heap since there is no point in them staying.

So, in BackgroundWorker class, I set the caller to null right after calling the interface method:

public class BackgroundWorker extends AsyncTask<String,Void,String> {

    MyCallBack caller;   // object to be called back when data arrives

    private static int instances_counter = 0;   // counts bw instances

    BackgroundWorker(MyCallBack cb) {
      caller = cb;
      instances_counter ++;
      Log.d("new bw object created", "current count: " + instances_counter );
    }

   @Override protected void finalize() throws Throwable {
      instances_counter --;
      Log.d("bw object destroyed","remaining count: " + instances_counter );
   }
    
   @Override String doInBackground(...) {...} // satisfy AsynTask 
   // and do the actual URL post/get stuff then forward the output
    // to onPostExecute() method

   
   @Override void onPostExecute(String s) {
      // I can check if caller is null first, but you get the idea.
      caller.data_received(s);

      caller = null;   // just so that this instance has no pointer to activity
   }
}

Now when I click the button multiple times, I can see the objects getting created, one by one. and I get the data each time inside my activity.

D/new bw object created: current count: 1

D/got data: {"results":[{"gender":"female" ...


D/new bw object created: current count: 2

D/got data: {"results":[{"gender":"male" ...


D/new bw object created: current count: 3

D/got data: {"results":[{"gender":"female" ...


D/new bw object created: current count: 4

D/got data: {"results":[{"gender":"female" ...


D/new bw object created: current count: 5

D/got data: {"results":[{"gender":"male" ...

But they are never destroyed. I am afraid that I am making a memory leak somewhere and that my app will at one point crash due to poor memory management on my part.

Why these instances are not being destroyed?

Community
  • 1
  • 1
Ahmad
  • 12,336
  • 6
  • 48
  • 88

1 Answers1

1

A log output in the finalize method is not a proper way to determine if an object causes memory leaks or not.

The only part that might trigger doubts is the fact the your AsyncTask instances hold a strong reference not just to any callback but to your entire Activity.

Still, the Garbage Collector is well trained to handle circular references anyway, but I admit I don't know whether it will wait until the Activity is gc'ed to collect the AsyncTask's as well or it will do it as soon as it can.

In any case, you can wrap your Callback references inside a WeakReference<T>

private WeakReference<MyCallBack> caller;   // object to be called back when data arrives
// ...
caller = new WeakReference<>(cb);
// ...
if (caller.get() != null)
     // call callback

and it should give you a major sense of security that the tasks will be collected as soon as memory is needed.

But I also want the consumed/done/already called back instances to go away, and leave the heap since there is no point in them staying.

This is honestly not up to you to decide.

payloc91
  • 3,724
  • 1
  • 17
  • 45