Am I using AsyncTask correctly?

Question

I was reading the book "Professional Android" and they say the following regarding AsyncTask:

It's important to note that Async Tasks have no built-in understanding of the life cycle of the components they're running with. This means that if you are creating an Async Task in an Activity, to avoid memory leaks you should define it as static (and ensure it doesn't hold a strong reference to an Activity or its Views).

To test Async Task I wrote the following code that should reverse a string in the background and show the string being built gradually by updating a TextView. Am I using strong reference here?

package com.example.leo.test01;

import android.os.AsyncTask;
import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.util.Log;
import android.view.View;
import android.widget.ProgressBar;
import android.widget.TextView;

public class MainActivity extends AppCompatActivity {

    private static final String TAG = "MainActivity";

    private TextView textView;
    private ProgressBar progressBar;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        textView = findViewById(R.id.text_view_reversed_string);

        new reverseStringAsync(textView).execute("ABCDEFGHIJKLMNOPQRSTUVWXYZ");
    }

    private static class reverseStringAsync extends AsyncTask<String, String, String> {

        private TextView textView;

        reverseStringAsync(TextView textView) {
            this.textView = textView;
        }

        @Override
        protected void onPreExecute() {
            Log.d(TAG, "onPreExecute()");
            textView.setText("");
        }

        @Override
        protected String doInBackground(String... strings) {
            Log.d(TAG, "doInBackground()");
            int n = strings[0].length();
            StringBuilder stringBuilder = new StringBuilder();
            for (int i = 1; i  <= n; i++) {
                stringBuilder.append(strings[0].charAt(n - i));
                publishProgress(stringBuilder.toString());
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
            return stringBuilder.toString();
        }

        @Override
        protected void onProgressUpdate(String... values) {
            Log.d(TAG, "onProgressUpdate()");
            textView.setText(values[0]);
        }

        @Override
        protected void onPostExecute(String s) {
            Log.d(TAG, "onPostExecute()");
            textView.setText(s);
        }
    }
}

Answer 1

You really shouldn't be sleeping on an AsyncTask like that. The reason- all AsyncTasks share the same thread (unless you call executeOnExecutor). This means that if one task is running, no other AsyncTask in your system can run until its finished. As a result, an AsyncTask should be very quick and isolated. Something that's going to be sleeping N times for a second each time should not be an async task.

For something like this, the right answer is probably just posting a delayed message to a Handler and running it on the UI thread with each delay.

For something that's actually doing CPU intensive work but needs to sleep like that, use a Thread.

Answer 2

Yes your async task is capturing a strong reference to the Activity's TextView. Perhaps to avoid memory leaks, you should wrap the TextView instance with a WeakReference<TextView>.

More on that over here : How to use WeakReference in Java and Android development?