33

I've been working with AsyncTasks in Android and I am dealing with an issue.

Take a simple example, an Activity with one AsyncTask. The task on the background does not do anything spectacular, it just sleeps for 8 seconds.

At the end of the AsyncTask in the onPostExecute() method I am just setting a button visibility status to View.VISIBLE, only to verify my results.

Now, this works great until the user decides to change his phones orientation while the AsyncTask is working (within the 8 second sleep window).

I understand the Android activity life cycle and I know the activity gets destroyed and recreated.

This is where the problem comes in. The AsyncTask is referring to a button and apparently holds a reference to the context that started the AsyncTask in the first place.

I would expect, that this old context (since the user caused an orientation change) to either become null and the AsyncTask to throw an NPE for the reference to the button it is trying to make visible.

Instead, no NPE is thrown, the AsyncTask thinks that the button reference is not null, sets it to visible. The result? Nothing is happening on the screen!

Update: I have tackled this by keeping a WeakReference to the activity and switching when a configuration change happens. This is cumbersome.

Here's the code:

public class Main extends Activity {

    private Button mButton = null;
    private Button mTestButton = null;

    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        mButton = (Button) findViewById(R.id.btnStart);
        mButton.setOnClickListener(new OnClickListener () {
            @Override
            public void onClick(View v) {
                new taskDoSomething().execute(0l);
            }
        });
        mTestButton = (Button) findViewById(R.id.btnTest);   
    }

    private class TaskDoSomething extends AsyncTask<Long, Integer, Integer> 
    {
        @Override
        protected Integer doInBackground(Long... params) {
            Log.i("LOGGER", "Starting...");
            try {
                Thread.sleep(8000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            return 0;
        }

        @Override
        protected void onPostExecute(Integer result) {
            Log.i("LOGGER", "...Done");
            mTestButton.setVisibility(View.VISIBLE);
        }
    }
}

Try executing it and while the AsyncTask is working change your phones orientation.

dnkoutso
  • 6,041
  • 4
  • 37
  • 58
  • a bit misleading to write "keeping a WeakReference to the activity" and than not doing it in code. Even worse, further you say "and switching when a configuration change happens" which means really nothing...Switching what? – Ewoks Jul 14 '15 at 14:33

4 Answers4

23

AsyncTask is not designed to be reused once an Activity has been torn down and restarted. The internal Handler object becomes stale, just like you stated. In the Shelves example by Romain Guy, he simple cancels any currently running AsyncTask's and then restarts new ones post-orientation change.

It is possible to hand off your Thread to the new Activity, but it adds a lot of plumbing. There is no generally agreed on way to do this, but you can read about my method here : http://foo.jasonhudgins.com/2010/03/simple-progressbar-tutorial.html

jasonhudgins
  • 2,815
  • 1
  • 25
  • 20
3

If you only need a context and won't use it for ui stuff you can simply pass the ApplicationContext to your AsyncTask.You often need the context for system resources, for example.

Don't try to update the UI from an AsyncTask and try to avoid handling configuration changes yourself as it can get messy. In order to update the UI you could register a Broadcast receiver and send a Broadcast.

You should also have the AsyncTask as a separate public class from the activity as mentioned above, it makes testing a lot easier. Unfortunately Android programming often reinforces bad practices and the official examples are not helping.

Kheldar
  • 5,361
  • 3
  • 34
  • 63
2

This is the type of thing that leads me to always prevent my Activity from being destroyed/recreated on orientation change.

To do so add this to your <Activity> tag in your manifest file:

android:configChanges="orientation|keyboardHidden" 

And override onConfigurationChanged in your Activity class:

@Override
public void onConfigurationChanged(final Configuration newConfig)
{
    // Ignore orientation change to keep activity from restarting
    super.onConfigurationChanged(newConfig);
}
Mark B
  • 183,023
  • 24
  • 297
  • 295
  • 5
    Your approach works but do you have good counter-arguments to this approach? Why is it by default that the OS destroys and recreates the activity then? In other words what do I "lose" by doing what you propose? Btw, it worked as expected though. Thanks for the reply. – dnkoutso Jan 23 '10 at 21:27
  • As far as I know, the main thing you would lose would be the ability to have different Activity layouts for landscape and portrait mode. There may be other things that you would "lose" but I don't know what they are. – Mark B Jan 23 '10 at 21:57
  • 1
    Google generally recommends against this approach unless you know what you're doing. – emmby Dec 20 '10 at 23:11
  • 2
    I understand that an activity can also be finished due to an incoming call for example. If that is the case, you still would have the problem, right? – gschuager Jan 28 '11 at 16:26
  • 14
    I would advise to avoid this solution. Problem #1 - Activity can be restarted on configuration change. Besides orientation change this can be change of font size, locale etc. Problem #2 - the AsyncTask has a reference to the Activity. If you finish() the Activity, it will remain in memory until the AsyncTask completes. If user launches Activity, presses BACK, launches again etc., there can be several instances of the same Activity in memory and could lead to OOM error. Problem #3 - you can't use different resources for portrait / landscape orientation. – fhucho Oct 23 '11 at 21:45
2

To avoid this you can use the answer givin here: https://stackoverflow.com/a/2124731/327011

But if you need to destroy the activity (different layouts for portrait and landscape) you can make the AsyncTask a public class (Read here why it shouldn't be private Android: AsyncTask recommendations: private class or public class?) and then create a method setActivity to set the reference to the current activity whenever it is destroyed/created.

You can see an example here: Android AsyncTask in external class

Community
  • 1
  • 1
neteinstein
  • 17,529
  • 11
  • 93
  • 123