Background image loader AsyncTask Usually, AsyncTasks are meant to be one-shot, i.e. you start it, it fetches some data, displays the result and dies. However, I'm now using a long-running AsyncTask to load images as the need arises. So far it's working great, but I'd just like to know if this is good practice &amp; if there's any threading detail that I missed. Here's my slimmed down implementation:

public class ImageLoader extends AsyncTask&lt;Void, Object, Void&gt; {
    private final List&lt;String&gt; pendingURLs = new Vector&lt;String&gt;();
    private Thread backgroundThread;

    @Override
    protected Void doInBackground(Void... params) {
        this.backgroundThread = Thread.currentThread();
        while (!backgroundThread.isInterrupted()) {
            String url = null;
            synchronized(this) {
                if (pendingURLs.size() &gt; 0) {
                    url = pendingURLs.remove(0);
                }
            }
            if (url != null) {
                // fetch image...
                publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
            }
            try {
                synchronized(this) {
                    if (pendingURLs.size() == 0) {
                        wait();
                    }
                }
            } catch (InterruptedException e) {
                break;
            }
        }
    }

    public syncrhonized void addUrl(String url) {
        pendingURLs.add(url);
        notify();
    }

    public Thread getBackgroundThread() {
        return backgroundThread;
    }

}


Then, in my Fragment#onCreate() I create a new instance and execute it like this:

mLoader = new ImageLoader();
if (Build.VERSION.SDK_INT &gt;= Build.VERSION_CODES.HONEYCOMB) {
    mLoader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
} else {
    mLoader.execute();
}


Finally, in my Fragment#onDestroy() I do:

if (mLoader != null &amp;&amp; mLoader.getBackgroundThread() != null) {
    mLoader.getBackgroundThread().interrupt(); // this will cause it to break out of the while loop and finish the thread
}
For starters, you should replace your Vector with an ArrayBlockingQueue .  Then you can drop the wait and the null check.  Can also then remove synchronization on the add method since its handled by the data structure.

private final BlockingQueue&lt;String&gt; pendingURLs = new ArrayBlockingQueue&lt;String&gt;(64);
protected void doInBackground(Void... params) {
    this.backgroundThread = Thread.currentThread();
    while (!backgroundThread.isInterrupted()) {
        String url = pendingURLs.take();
        // fetch image...
        publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
    }

public void addUrl(String url) {
    pendingURLs.put(url);
}


You should probably replace it with just a new thread instead of using asynctask at all.

public class ImageLoader implements Runnable {
  private final BlockingQueue&lt;String&gt; pendingURLs = new ArrayBlockingQueue&lt;String&gt;();

  public void run() {
    while (true) {
        String url = pendingURLs.take(); 
        // fetch image...
        publishProgress(url, fetchedBitmap); // in onProgressUpdate I call a listener that can be implemented by any Fragment or other component
    }
  }

  public void addUrl(String url) {
    pendingURLs.put(url);
  }
}


Then you can create it in your initialization somewhere, ie

 ImageLoader loader = new ImageLoader();
 new Thread(loader, "BackgroundLoader").start();