1

The following code updates a TextView till a certain condition evaluates to false and then the Handler postDelayed is not called further.

However if the activity is destroyed, it'll try to update a null TextView, what is the right way to handle this? I know I could put a defensive null check on TextView but that is still not truly thread safe.

@Override
protected void onCreate(Bundle savedInstanceState) {
     Handler durationHandler = new Handler();
     durationHandler.postDelayed(updateSeekBarTime, 50);
}

private Runnable updateSeekBarTime = new Runnable() {
            public void run() {

                timeElapsed = mediaPlayer.getCurrentPosition();

                double timeRemaining = finalTime - timeElapsed;

                timeLeft.setText(String.format("%d", TimeUnit.MILLISECONDS.toSeconds((long) timeRemaining)));

                if (timeRemaining >= 1000)
                    durationHandler.postDelayed(this, 200);
            }
        };

In other words the updateSeekBarTime can at any execution point try to access data members of an already destroyed activity, how to prevent that?

2cupsOfTech
  • 5,953
  • 4
  • 34
  • 57

3 Answers3

2

Start your handler in onResume(). In onPause() stop the handler with

handler.removeCallbacksAndMessages(null); //null removes everything
ElDuderino
  • 3,253
  • 2
  • 21
  • 38
  • I think .removeCallbacksAndMessages will get rid of any Runnables that have not started execution. My question is very specific about how to prevent accessing a de-allocated TextView – 2cupsOfTech Aug 18 '15 at 18:37
  • Guess you won't get around a null check then. Or do it like this guy : http://stackoverflow.com/a/5844433/2001247 – ElDuderino Aug 18 '15 at 19:50
  • as the person pointed out there, a kill switch can only prevent from starting a Runnable but not in the middle of Runnable – 2cupsOfTech Aug 18 '15 at 20:02
  • Wrong, what's inside run() is never executed after the kill switch... private void run() { if(killMe) return; /* do your work */ } – ElDuderino Aug 18 '15 at 20:38
  • so lets say killMe becomes true after the if(killMe) condition executes then the rest of the code inside run() will still execute – 2cupsOfTech Aug 18 '15 at 20:49
  • So what again is the problem you have with the null check? – ElDuderino Aug 18 '15 at 21:29
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/87315/discussion-between-2cupsoftech-and-elduderino). – 2cupsOfTech Aug 18 '15 at 21:37
1

I'm building my own Music Player app and I use

durationHandler.removeCallbacks(updateSeekBarTime);

in onStop(). It works for me.

EDIT:

The above line is helped by the fact that I prevent the Activity from being destroyed by using

@Override
public void onBackPressed() {
    moveTaskToBack(true);
}

This ensures that the Activity is minimized instead of destroyed so that when opened again, it's very snappy.

EDIT 2:

private Runnable updateSeekBarTime = new MyRunnable();

private class MyRunnable extends Runnable {
        private boolean dontWriteText = false;
        @Override
        public void run() {

            timeElapsed = mediaPlayer.getCurrentPosition();

            double timeRemaining = finalTime - timeElapsed;

        if(!dontWriteText)
            timeLeft.setText(String.format("%d", TimeUnit.MILLISECONDS.toSeconds((long) timeRemaining)));

            if (timeRemaining >= 1000)
                durationHandler.postDelayed(this, 200);
        }
        public void dontWriteText() {
            dontWriteText = true;
        }
    };

Then call updateSeekBarTime.dontWriteText() in onDestroy() or onStop(). I'd prefer onStop().

David Heisnam
  • 2,463
  • 1
  • 21
  • 32
  • and that makes sense because any pending Runnables are removed from your handler queue but what about the one that is still possibly executing in parallel to main UI thread? – 2cupsOfTech Aug 18 '15 at 19:50
  • so your activity onDestroy() is never called? How about activity rotation? Unless system kills your activity in which case onDestroy() will be called I think. – 2cupsOfTech Aug 18 '15 at 20:25
  • @2cupsOfTech I've absolutely never encountered a problem even when I rotate screen, even with 100ms delay in postDelayed(). Anyway, did you see my updated answer? It should work for your needs. – David Heisnam Aug 18 '15 at 20:28
  • Yes I saw your answer and I think its a clever/hacky way of addressing the issue however check the answer I posted. – 2cupsOfTech Aug 19 '15 at 20:15
  • @2cupsOfTech I was considering suggesting WeakReference yesterday, but decided against it because you're not passing any reference, weak or strong, to the runnable. Anyway, glad to know you found your own answer. – David Heisnam Aug 19 '15 at 20:19
1

So after some code searching and reading blogs I found the answer in sample code of Communicating with the UI Thread

Even though you can and should be removing callbacks from handler:

handler.removeCallbacksAndMessages(null)

But the above does not prevent the existing running Thread from accessing a destroyed Activity or its views.

The answer I was looking for is WeakReference.

Create a weak reference to the TextView or UI element that you'll access. The weak reference prevents memory leaks and crashes, because it automatically tracks the "state" of the variable it backs. If the reference becomes invalid, the weak reference is garbage-collected. This technique is important for referring to objects that are part of a component lifecycle. Using a hard reference may cause memory leaks as the value continues to change; even worse, it can cause crashes if the underlying component is destroyed. Using a weak reference to a View ensures that the reference is more transitory in nature.

You should still check for null reference but now the view will be set to null by the active thread/runnable so you will not face a race-condition.

2cupsOfTech
  • 5,953
  • 4
  • 34
  • 57
  • If you post the runnable on the UI thread (which you should do) and then remove it in a lifecycle event, there is no way the runnable will be executed, because event and runnable cannot run together *on the same* thread. Using WeakReference is a good idea, but there is more to this approach, namely making the runnable a static inner class and adding a flag mIsDestroyed to the activity. Don't mix up destroyed with garbage collected. – Gil Vegliach Aug 19 '15 at 20:38
  • @GilVegliach pleas see the comments in other answers – 2cupsOfTech Aug 22 '15 at 11:46
  • I read them, and I still think you are mixing up things. For starters, destroyed activity != activity eligible to be gc'd. Even in the statement of your question, if you hold on a destroyed activity, a textview inside it won't be null when the runnable runs. That said, using weak references to *let the activity be gc'd* is a good idea, just keep in mind that *destroyed* here (in Android) has a very specific meaning, see: http://developer.android.com/reference/android/app/Activity.html#isDestroyed() – Gil Vegliach Aug 22 '15 at 16:51
  • @GilVegliach when I say 'destroyed activity' I mean a reference is not being held to an activity or to any of its views. It would actually be a memory leak if I hold reference to any of its views: http://android-developers.blogspot.com/2009/01/avoiding-memory-leaks.html – 2cupsOfTech Aug 23 '15 at 01:33
  • exactly, you mean 'eligible for gc', not destroyed. In this case, make also sure to turn the anonymous inner runnable into a static inner class or you will leak nevertheless. – Gil Vegliach Aug 23 '15 at 10:46