6

In the latest version of my app, some users are experiencing a crash that I'm unable to reproduce. Currently only Samsung devices running Lollipop are having the issue, but that might just be coincidence. After analyzing the stack trace and relevant code, I think that I may have found the culprit. To test my assumption, I simplified the code to the snippet below:

public class TestActivity extends AppCompatActivity {

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

        Button b = new Button(this);
        b.setText("Click me!");
        b.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                new Handler().post(new Runnable() {
                    @Override
                    public void run() {
                        // This is the callback method
                        Log.d("TAG", "listenerNotified");
                    }
                });
            }
        });

        setContentView(b);
    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        Log.d("TAG", "onDestroy");
    }

}

Every time I test the above application by first tapping the Click me button and then the back button, listenerNotified is printed to the console before onDestroy().

I'm however not sure if I can rely on this behavior. Does Android make any guarantees about the above scenario? Can I safely assume that my Runnable will always be executed before onDestroy() or is there a scenario where that won't be the case? In my real app, there is of course a lot more happening (like other threads posting to the main thread and more operations happening in the callback). But this simple snippet seemed sufficient to demonstrate my concern.

Is it every possible (possibly due to the influence of other threads or callbacks posted to the main thread) that I get the debug output below?

D/TAG: onDestroy
D/TAG: listenerNotified

I would like to know this, since that outcome being possible would explain the crash.

Onik
  • 19,396
  • 14
  • 68
  • 91
s1m0n
  • 7,825
  • 1
  • 32
  • 45
  • 1
    Why are you posting the runnable through a handler? Meanwhile you can take a look at http://stackoverflow.com/questions/31432014/onclicklistener-fired-after-onpause – Robert Estivill Sep 18 '16 at 22:04
  • 1
    When you have asynchronous callbacks like this you should always check in the callback if the `Activity` is still alive before processing the callback. The easiest way to do this is to call `isFinishing()`, which returns `true` if the `Activity` is no longer "active". – David Wasser Sep 20 '16 at 18:53

3 Answers3

3

Is it possible for a callback method to be called after onDestroy()?

Yes.

Let's change a bit your sample code regarding posting a Runnable to the Handler. I also assume (according to your description) that you may have multiple Runnables posted to the main thread, so at some point there might be a queue of Runnables which brings me to a delay in the experiment below:

public void onClick(View view) {
    new Handler().postDelayed(new Runnable() {
        @Override
        public void run() {
            // This is the callback method
            Log.d("TAG", "listenerNotified");
        }
    }, 3000);
}

Now push the button b, then press the back button and you should see the output in question.

Might it be the reason of your app crash? It's hard to say without seeing what you got. I'd just like to note that when new Handler() is instantiated on a thread (the main thread in your case), the Handler is associated with the Looper's message queue of the thread, sending to and processing Runnables and messages from the queue. Those Runnables and messages have a reference to the target Handler. Even though Activity's onDestroy() method isn't a "destructor", i.e. when the method returns the Activity's instance won't be immediately killed (see), the memory cannot be GC-ed because of the implicit reference* to the Activity. You'll be leaking until the Runnable will be de-queued from the Looper's message queue and processed.

A more detailed explanation can be found on How to Leak a Context: Handlers & Inner Classes

* Instance of anonymous inner class Runnable has a refers to an instance of anonymous inner class View.OnClickListener that, in its turn, has a reference to the Activity instance.

Community
  • 1
  • 1
Onik
  • 19,396
  • 14
  • 68
  • 91
  • `Handler` does not have a reference to `Activity`. What makes you think so? – David Wasser Sep 20 '16 at 18:46
  • @David Wasser Many thanks for pointing out the mistake I've made. – Onik Sep 20 '16 at 19:52
  • 1
    I've read that article about leaking memory with `Handler` and `Context`. In practice this isn't usually a real problem unless you have a `static` variable that is referencing a dead `Context`. That would be a real memory leak (ie: memory that is never reclaimed). Most of the time these "leaks" are short-lived, so they aren't real leaks. As you've stated "You'll be leaking until the `Runnable` will be dequed..." which is usually a very short time. Not anything to worry about. – David Wasser Sep 20 '16 at 20:22
1

You might need to consider not just posting delayed runnable to handler. You might encounter issues when your running task into a separate thread and your activity is already destroyed. You can do and implement like this.

your class activity
{
  Handler mHandler;

  .. onCreate ()
  {
    mHandler = new Handler();
  }

  .. onDestory ()
  {
    if (mHandler != null)
    {
      mHandler.removeCallbacksAndMessages(null);
      mHandler = null;
    }
  }

  private void post (Runnable r)
  {
    if (mHandler != null)
    {
      mHandler.post(r);
    }
  }
}

By this, any pending task on handler message queue will be destroyed upon activity is destroyed.

Only to make consider that you are aware that you dont need to run any task after the activity is destroyed.

Keyfe Ang
  • 386
  • 3
  • 4
0

The answer comes to be "yes".And this may cause Memory Leak by the way.

Qian Sijianhao
  • 564
  • 7
  • 19