New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert AsyncTask to IntentService #3299

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@yulin2

yulin2 commented Jun 1, 2015

Hi TextSecure developers, I'm doing research on Android async programming, particularly on
AsyncTask and IntentService. AsyncTask can lead to memory leak and losing task result when GUI is recreated during task running (such as orientation change). This article describes the problems very well.

We discussed with some Android experts and they agree with this issue, and claim that AsyncTask can be considered only for short tasks (less than 1 second). However, using IntentService (or AsyncTaskLoader) can avoid such problems since their lifecycles are independent from Activity/Fragment.

In TextSecure, the AsyncTasks are declared as non-static inner classes of Activity/Fragment. So the above issue can occur (especially for relatively long tasks).

I refactored 9 AsyncTasks in TextSecure to IntentService (in 8 separate commits), with the help of a refactoring tool we developed. Do you think using IntentService should be better in some of these 9 scenario? Do you want to merge some commits in this pr? Thanks.

@moxie0

This comment has been minimized.

Show comment
Hide comment
@moxie0

moxie0 Jun 2, 2015

Member

I think we'd probably like to avoid using IntentService. Most of these cases would probably be better addressed just by making the AsyncTask inner class static, and using an event bus to communicate the result back to the activity.

Member

moxie0 commented Jun 2, 2015

I think we'd probably like to avoid using IntentService. Most of these cases would probably be better addressed just by making the AsyncTask inner class static, and using an event bus to communicate the result back to the activity.

@yulin2

This comment has been minimized.

Show comment
Hide comment
@yulin2

yulin2 Jun 2, 2015

Thanks for the reply! Are there any specific reasons that you want to avoid using IntentService?

On the other hand, since many AsyncTasks access fields of Activity, I don't think you can make them into static inner classes. Otherwise they cannot be compiled, right?

yulin2 commented Jun 2, 2015

Thanks for the reply! Are there any specific reasons that you want to avoid using IntentService?

On the other hand, since many AsyncTasks access fields of Activity, I don't think you can make them into static inner classes. Otherwise they cannot be compiled, right?

@moxie0

This comment has been minimized.

Show comment
Hide comment
@moxie0

moxie0 Jun 3, 2015

Member

IntentService is too cumbersome and heavy handed. If your inner class isn't static (whether an asynctask or a loader), it will retain a reference to the parent activity, which I assume is what you're trying to avoid. If the task references an instance variable of the enclosing class, that needs to be passed as an argument instead or handled by the callback.

Member

moxie0 commented Jun 3, 2015

IntentService is too cumbersome and heavy handed. If your inner class isn't static (whether an asynctask or a loader), it will retain a reference to the parent activity, which I assume is what you're trying to avoid. If the task references an instance variable of the enclosing class, that needs to be passed as an argument instead or handled by the callback.

@moxie0 moxie0 closed this Jun 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment