5

i've been constantly frustrated by this and i can't find a good answer, so hoping someone here can offer guidance.

i have a fragment that uses AsyncTask quite extensively. i'm constantly plagued by bugs where the fragment calls getActivity(), which returns null. i assume these are happening because some method in the fragment are invoked before the activity is attached, or after it is detached.

what's the correct way to handle this in my code? i don't want to have this idiom littered all over the place,

Activity activity = getActivity();
if (activity != null) { // do something }

looking at the docs for Fragment, i can come up with many possible hooks to solve this: isDetached(), onActivityCreated(), onAttach(), isResumed(), and so on. what is the right combination?

EDIT:

A few people have suggested canceling tasks when paused, but this implies that the standard idiom,

new AsyncTask<...>.execute();

cannot be used. It implies that every exec'd AsyncTask needs to be tracked to completion, or canceled. I have simply never seen that in example code from Google or elsewhere. Something like,

private final Set<AsyncTask<?>> tasks = new HashSet<>;
...
AsyncTask<?> t = new AsyncTask<...>() {
  ...
  public void onPostExecute(...) {
    tasks.remove(this);
    ...
  } 
}
tasks.add(t);
t.execute();
...
@Override
public void onPause() {
  for (AsyncTask<?> t: tasks) {
    t.cancel();
  }
  tasks.clear();
}
Jeffrey Blattman
  • 22,176
  • 9
  • 79
  • 134
  • Definitely not an answer, but... Your bug is architectural. I don't know exactly what you are doing with AsyncTasks, but I can say that if getActivity returns null while one of them is running, you shouldn't be using AsyncTask. Whatever you are doing in the task, should be done in a Service. An activity/fragment that needs to know find state can ask the Service to get. Check out the IntentService: it is easy to use and much simpler than an AsyncTask (ok, now wait: it is simpler than an AsyncTask actually **is**, not simpler than it appears). – G. Blake Meike Feb 26 '13 at 22:09
  • @G.BlakeMeike thanks for the feedback. we actually have both patterns in the app: `IntentService` and `AsyncTask`. i much prefer the call semantics of `AsyncTask`. re: wrong architecture- i don't know if i agree. any time you need a context you must call `getActivity()`. so needing to obtain say a string in onPostExecute() means i have the wrong arch? – Jeffrey Blattman Feb 26 '13 at 22:32
  • Well.. yeah, I guess that is pretty much what I'm saying. The whole deal with an AsyncTask is that it has a lifecycle that is totally detached from activities. You cannot count on *any* Activity context being around, at any given time in an AT's life. Have you considered passing the ApplicationContext to the AT's constructor? – G. Blake Meike Feb 26 '13 at 22:36
  • re: passing AC- i was giving a contrived example with the string. normally i'm updating the UI when the task completes. that's not allowed if the fragment is detached. more simply, the whole purpose of AT is to provide a background thread + a callback to the UI thread to update the UI when it's finished. if i can't do that, the whole concept is broken. – Jeffrey Blattman Feb 26 '13 at 22:44
  • Yeah, if that is the purpose, then the concept is broken. I think of the purpose of the AT is to do something *incredibly* short lived: 2-3 seconds at the very very most, on a bg thread. Something like read a local DB or render an image. If the Activity/Fragment isn't around when it gets done, it should be willing to just give up. If it can't do that, I think Service. BTW, there's code here that I've come to consider wrong-headed. Might be useful, tho: https://github.com/bmeike/OSCON – G. Blake Meike Feb 26 '13 at 23:30
  • yes that's what i'm asking. what's the right pattern for determining when to give up. even if it's only a 1s bg task, you still have to prepare for that activity not being around. – Jeffrey Blattman Feb 27 '13 at 04:13
  • Right. IMO, ATs should be local, trivial and, best, stateless. A Loader moves DB state to the Activity. If the Activity goes away, it quits, w/no harm. Same goes for rendering an image. Pushing to a DB works because it can complete w/only a ContentResolver. Inbound it has to give up if the Activity is gone; outbound it has to complete independent of the Activity. – G. Blake Meike Feb 27 '13 at 14:38

3 Answers3

2

Try to cancel your AsyncTasks in the onPause or onStop methods. That will prevent the onPostExecute from being called when the Fragment is not active anymore (getActivity() returns null).

Or you could check if the Fragment is attached by calling this.isAdded() in your Fragment.

nhaarman
  • 98,571
  • 55
  • 246
  • 278
  • thanks. the problem is i'd need to keep a handle to every `AsyncTask` that i execute. e.g., keep a stack of them, pop them when `onPostExecute()` is called or when i cancel them. there is no `isAttached()` method, only `isDetached()`, which is only true when the fragment has been detached, not when it has not been attached yet (see javadocs). – Jeffrey Blattman Feb 26 '13 at 21:16
  • I've actually changed the `isAttached` method to `isAdded` directly after posting the answer ;) See also http://stackoverflow.com/questions/10919240/fragment-myfragment-not-attached-to-activity – nhaarman Feb 26 '13 at 21:17
  • Also, usually I try to keep the number of `AsyncTask`s per `Activity`/`Fragment` below 3, so that is doable. – nhaarman Feb 26 '13 at 21:19
1

I could not find a good solution. In summary, either use a Loader, or check that getActivity() does not return null before using it. I looked into using Loaders, but the pattern makes a lot of assumptions about the structure of the app and the nature of data retrieval that didn't work for me.

Nimantha
  • 6,405
  • 6
  • 28
  • 69
Jeffrey Blattman
  • 22,176
  • 9
  • 79
  • 134
0

In terms of coordinating lifecycles, I keep onActivityCreated as a mental benchmark - it marks the point at which the underlying activity has finished its own onCreate. Prior to that I do not believe there is an activity to getActivity() from.

That get activity is returning null sounds like either you're calling getActivity() too early (i.e. before it is created) or too late (i.e. when it stopped interacting with the fragment). Stopping your tasks in onPause() would prevent getActivity from returning null since it would cut off the task once the fragment stopped interacting with the underlying activity becuase the activity itself was paused. I think waiting for onStop() may be too late since, if the task were to still be running when the underlying activity paused it may still reutrn null.

Rarw
  • 7,645
  • 3
  • 28
  • 46