Is it safe to keep a strong reference to a Fragment in an AsyncTask?

Question

Since it's not recommended to keep a strong reference to a Context in a task (the context may get destroyed while the task is still running, but kept in memory by the task), I was wondering if the same applies to Fragments?

Fragments manage their activity reference, and support being retained via setRetainInstance. Can I assume that e.g. creating a non-static inner AsyncTask in a Fragment is safe in terms of not risking to leak $this?

score 6 · Accepted Answer · edited May 22 '12 at 20:01

6

It's generally bad methodology to keep references between threads, and an AsyncTask is something like a thread.

It's okay, as long as you make sure that you dereference it when you are done using it.

Otherwise, you could get memory leaks.

In this case, it's okay because your Fragment is in the context of AsyncTask. When the task is done, it will lose that reference.

If this were being done in a Service, it would be a very bad idea.

edited May 22 '12 at 20:01

Jeffrey Blattman

22,176
9
79
134

answered May 22 '12 at 19:53

Codeman

12,157
10
53
91

2

The risk is in this case if you keep the reference to the asynctask when the Fragment is dead. The user could say do an action that pops the fragment from the stack. In this case, you usually wouldn't want the Fragment's state to exist anymore. If you wanted the asynctask's state because it contained extra data you wanted to use, then it would keep the reference to the Fragment even though the Fragment isn't supposed to exist anymore. I do believe it will be detached from the activity since it went through the normal lifecycle, but it would still "exist". – DeeV May 22 '12 at 20:01
@DeeV: Yep, that's exactly what I was worried about – mxk May 22 '12 at 20:04
Could you simply get away with assigning it to a WeakReference and canceling the AsyncTask if the Fragment becomes null? – DeeV May 22 '12 at 20:15
I'm not entirely sure about WeakReference (I haven't used it), but cancelling the AsyncTask if the fragment becomes null is one way to handle your problem. – Codeman May 22 '12 at 20:19
There's an example on Commonsware on how to detach the AsyncTask. But there's another thing which may be problematic - so called memory boosters - explicitly called garbage collectors. I'm not sure how it would affect the AsyncTask, though – Michał Klimczak May 22 '12 at 20:42
Well yes I could use a WeakReference, or simply clear the strong reference and detach the task upon fragment destruction. I was just wondering if there was a way to relieve one of the extra boilerplate. Guess not, or use `Loader` as Michal suggests. – mxk May 22 '12 at 21:01

Michał Klimczak · Answer 2 · 2012-05-25T21:23:23.113

Phoenixblade9's answer is correct, but to make it full I'd add one thing.

There's a great replacement for AsyncTask - AsyncTaskLoader, or Loaders generally. It manages its lifecycle according to the context from which it's been called (Activity, Fragment) and implements a bunch of listeners to help you separate the logic of a second thread from the ui thread. And it's generally immune to leaking context.

And don't bother the name - it's good for saving data as well.

As promised I'll post my code for AsyncTaskLoader withg multiple objects returned. The loader goes something like this:

public class ItemsLoader extends AsyncTaskLoader<HashMap<String, Object>>{

HashMap<String, Object> returned;
ArrayList<SomeItem> items;
Context cxt;

public EventsLoader(Context context) {
    super(context);
    //here you can initialize your vars and get your context if you need it inside
}

@Override
public HashMap<String, Object> loadInBackground() {


    returned = getYourData();

    return returned;

}

@Override
public void deliverResult(HashMap<String, Object> returned) {
    if (isReset()) {
        return;
    }

    this.returned = returned;

    super.deliverResult(returned);
}

@Override
protected void onStartLoading() {
    if (returned != null) {
        deliverResult(returned);
    }

    if (takeContentChanged() || returned == null) {
        forceLoad();
    }
}

@Override
protected void onStopLoading() {
    cancelLoad();
}

@Override
protected void onReset() {
    super.onReset();

    onStopLoading();

    returned = null;
}

In the getYourData() function I get both the server message code or some other error code and an ArrayList<SomeItem>. I can use them in my Fragment like this:

public class ItemListFragment extends ListFragment implements LoaderCallbacks<HashMap<String, Object>>{

private LoaderManager lm;

@Override
public void onActivityCreated(Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);

    lm = getLoaderManager();

    Bundle args = new Bundle();
args.putInt("someId", someId);
lm.initLoader(0, args, this);
}


@Override
public Loader<HashMap<String, Object>> onCreateLoader(int arg0, Bundle args) {
    ItemsLoader loader = new ItemsLoader(getActivity(), args.getInt("someId"));
    return loader;
}

@Override
public void onLoadFinished(Loader<HashMap<String, Object>> loader, HashMap<String, Object> data) {

    if(data!=null){ if(data.containsKey("items")){ 
        ArrayList<SomeItem> items = (ArrayList<EventItem>)data.get("items");

    } else { //error
        int error = 0;
        if(data.containsKey("error")){
            error = (Integer) data.get("error");
        }
            }

}

@Override
public void onLoaderReset(Loader<HashMap<String, Object>> arg0) {

}

Yeah good point; I haven't gotten around to use `Loader` yet, but a quick look suggests that it suffers from the same flaw as AsyncTask (http://stackoverflow.com/questions/3357477/is-asynctask-really-conceptually-flawed-or-am-i-just-missing-something), i.e. not providing a managed reference to the Activity/Fragment in the callbacks. — mxk, May 22 '12 at 21:03
But why would you need a reference to Activity from inside of Loader? That's the beauty of it - this nice separation. The work on ui thread happens only in the code of activity (through the LoaderCallbacks interface), so the loading (saving etc) happens on the second thread, and updating the ui - on ui thread. The only flaw I can think of is the problem with updating ui DURING loading, but that's probably not so hard to do with an extensions of LoaderCallbacks. — Michał Klimczak, May 22 '12 at 22:20
What if you load two different things? You can't implement the same interface twice using different type variables, so the callbacks have to be implemented on a delegate, which then again must manage context references. Although I suppose one could create a loader composite which delivers a composite output that combines two different results. — mxk, May 23 '12 at 10:18
I even load HashMaps containing the Type of server response as an int and an ArrayList of custom objects. What you return is totally up to you. If I understood your concern properly. I can provide you with an example (on friday) if you want — Michał Klimczak, May 23 '12 at 11:23
Sure, that would be great, if you find the time! Much appreciated. — mxk, May 23 '12 at 11:40
Added the code. It's a bit long, but I tried to show you the essence. If anything isn't clear, feel free to ask — Michał Klimczak, May 25 '12 at 21:24

Is it safe to keep a strong reference to a Fragment in an AsyncTask?

2 Answers2