metinkale38 / prayer-times-android

A useful Application with a set of tools needed by every muslim.
https://play.google.com/store/apps/details?id=com.metinkale.prayer
Apache License 2.0
239 stars 105 forks source link

Misuse of AsyncTask #145

Closed cuixiaoyiyi closed 4 years ago

cuixiaoyiyi commented 4 years ago

According to our research, there are misuses about the following AsyncTask class: ( in com/metinkale/prayer/hadith/HadithFragment.java)

private SearchTask mTask;
//……
private class SearchTask extends AsyncTask<String, String, Boolean>

The problems are:

  1. It holds strong reference to the GUI element of Activity via HadithFragment , which can lead to memory leak when the Activity is destroyed and the AsyncTask did not finish.
  2. Its instance is not cancelled before the Activity is destroyed, which can lead to the wrong invocation of onPostExecute.

I think we can make following changes to fix the misuse problems:

  1. I think the GUI-related fields should be wrapped into WeakReference. Take private final Activity _context as example, it can be changed tp private final WeakReference<Activity> _context.
  2. Invoke cancel() in the onDestroy() method of Activities or Fragments.
metinkale38 commented 4 years ago

Even it might not be a good practice to do it like that, i see no problem here. It might be changed, when i consider to refactor the Hadith Feature, but for know it can stay like it is...

Thank you for reporting.