bparmentier / OpenBikeSharing

Shared bikes availability in your city
https://openbikesharing.org
Other
67 stars 56 forks source link

Misuse of AsyncTask #50

Open cuixiaoyiyi opened 5 years ago

cuixiaoyiyi commented 5 years ago

According to our research, there are misuses about the following AsyncTask class: ( tkj.android.homecontrol.mythmote. GestureBuilderActivity.BikeNetworksListActivity)

//……
new JSONDownloadTask().execute(apiUrl);
//……
private class JSONDownloadTask extends AsyncTask<String, Void, String>

The problems are:

  1. It is inner class. It holds strong reference to the GUI element of Activity( BikeNetworksListActivity ), which can lead to memory leak when the Activity is destroyed and the AsyncTask did not finish.
  2. Its instances are not cancelled before the Activity is destroyed, which can lead to the wrong invocation of onPostExecute.
  3. Its instance should be cancelled before the Activity is destroyed. But in the doInBackground method there are for or while loops without the call of isCancelled() . These loops may not be able to terminate when the task is cancelled. This will cause waste of resources or even flashback.

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 BikeNetworksListActivity _context as example, it can be changed to private final WeakReference<BikeNetworksListActivity> _context.
  2. Define JSONDownloadTask instance as a field instead of a local. Invoke cancel() in the onDestroy() method of Activities or Fragments.
  3. Every loop should check the status of AsyncTask to break the loop.
    while(condition1){
     if(isCancelled()){
          break;
     }
     //blablabla…
     while(condition2){
         if(isCancelled()){
              break;
         }
         //blablabla…
     }
    }

These are my suggestions above, thank you.