4
\$\begingroup\$

While I'm not aiming for a drop-in replacement for AsyncTask, I would like a utility class that accomplishes some of the same goals.

Considering the criticisms of AsyncTask, in many cases I just deferred responsibility to the user - if the work you're pushing to a background Thread needs to be aware of your Activity lifecycle, save a reference to the AsynchronousOperation and explicitly cancel it in onPause, and make sure you're checking for - and reacting to - cancellation in the performWorkInBackgroundThread method. If you want to send results back to the main thread, use the provided runOnUiThread method.

I believe I've used volatile and synchronized blocks correctly here, but would be happy to hear any feedback, even if it's just to point out that I've done it badly.

Other than that - does this feel useful? Safe? Any obvious places it could be improved?

import android.os.Handler;
import android.os.Looper;
import android.os.Process;

import java.util.concurrent.BlockingDeque;
import java.util.concurrent.LinkedBlockingDeque;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;

/**
 * Usage:
 * new AsynchronousOperation(){
 *   @Override
 *   public void performWorkInBackgroundThread(){
 *     // do stuff...
 *     // if you can break things into small steps, do so -
 *     // e.g., maybe download by looping through chunks instead of a library method
 *     // at every opportunity, check #shouldQuit, if true, bail out
 *     // (or use the convenience #quitIfCancelledOrInterrupted)
 *     // for any operation you want to publish, use runOnUiThread and a Runnable
 *     // this would be for things like AsyncTask.onProgressUpdate or AsyncTask.onPostExecute
 *     // remember to keep references to cancel if the operation depends on the lifecycle of a View or Activity
 *   }
 * }.start();
 */
public abstract class AsynchronousOperation implements Runnable {

  protected static final int INITIAL_POOL_SIZE = 1;
  protected static final int MAXIMUM_POOL_SIZE = Runtime.getRuntime().availableProcessors();
  protected static final int KEEP_ALIVE_TIME = 2;
  protected static final TimeUnit KEEP_ALIVE_TIME_UNIT = TimeUnit.SECONDS;
  protected static final BlockingDeque<Runnable> BLOCKING_DEQUE = new LinkedBlockingDeque<>();

  protected static ThreadPoolExecutor sThreadPoolExecutor;
  protected static Handler sHandler;

  protected volatile Thread mThread;

  private volatile boolean mCancelled;

  /**
   * Lazily instantiate the ThreadPoolExecutor, constructed with default values. If customization of these values is
   * required, override this getter method in the implementation subclass.
   *
   * This needs to be synchronized in case an AsynchronousOperation instance is started from within another thread -
   * we only ever want a single instances of this class, and it should only be accessible to AsynchronousOperation
   * instances.
   *
   * // TODO: this can probably be volatile, rather than synchronized
   *
   * @return A ThreadPoolExecutor instance used by all AsynchronousOperation instances.
   */
  protected ThreadPoolExecutor getThreadPoolExecutor() {
    synchronized(AsynchronousOperation.class) {
      if(sThreadPoolExecutor == null) {
        sThreadPoolExecutor = new ThreadPoolExecutor(
          INITIAL_POOL_SIZE,
          MAXIMUM_POOL_SIZE,
          KEEP_ALIVE_TIME,
          KEEP_ALIVE_TIME_UNIT,
          BLOCKING_DEQUE
        );
      }
      return sThreadPoolExecutor;
    }
  }

  /**
   * Lazily instantiate a new Handler on the main thread.  This Handler instance is common to and shared between
   * all AsynchronousOperation instances, and is only accessible to those instances.
   *
   * Synchronize it in case someone subclasses and calls getHandler from outside of #run.
   *
   * @return A Handler instance used by all AsynchronousOperation instances to communicate with the main thread.
   */
  protected Handler getHandler() {
    synchronized(AsynchronousOperation.class) {
      if(sHandler == null) {
        sHandler = new Handler(Looper.getMainLooper());
      }
      return sHandler;
    }
  }

  /**
   * This will usually be the Thread provided by the ThreadPoolExecutor when submitted to it, but since #run
   * is a public method, it might be the main thread (or any thread) if used inappropriately.  Assuming this does
   * not happen, you can be rely on this referencing the background Thread provided to it.
   *
   * @return The Thread that owned this instance the moment #run was invoked.
   */
  public Thread getThread() {
    return mThread;
  }

  /**
   * Cancels an operation.
   *
   * This is neither synchronized nor an AtomicBoolean because the boolean primitive for the cancelled flag is
   * volatile and only ever set to true (never set back to false), which should be thread-safe here.
   *
   * Cancellation by itself will attempt to interrupt the background thread this worker is on, but by itself will
   * not interrupt any work being performed - the user should test for cancellation frequently within the
   * #performWorkInBackgroundThread method.
   *
   * @param mayInterrupt True if cancelling this operation should also interrupt its owner Thread.
   * @return True if the operation was cancelled (and had not previously been cancelled).
   */
  public boolean cancel(boolean mayInterrupt) {
    if(mayInterrupt && mThread != null) {
      mThread.interrupt();
    }
    boolean alreadyCancelled = mCancelled;
    mCancelled = true;
    return !alreadyCancelled;
  }

  /**
   * @return True if this AsynchronousOperation has been explicitly cancelled.
   */
  public boolean isCancelled() {
    return mCancelled;
  }

  /**
   * @return True if this AsynchronousOperation instance's owner thread has been interrupted.
   */
  public boolean isInterrupted() {
    return mThread != null && mThread.isInterrupted();
  }

  /**
   * @return True if this AsynchronousOperation has been explicitly cancelled or its owner thread has been interrupted.
   */
  public boolean isCancelledOrInterrupted(){
    return isCancelled() || isInterrupted();
  }

  /**
   * Tests for explicit cancellation or thread interruption - if either are true, it cancels and offers another
   * opportunity to interrupt the owner thread.
   *
   * @param mayInterrupt True if cancelling this operation should also interrupt its owner Thread.
   * @return True if this AsynchronousOperation has been explicitly cancelled or its owner thread has been interrupted.
   */
  public boolean quitIfCancelledOrInterrupted(boolean mayInterrupt){
    boolean shouldQuit = isCancelledOrInterrupted();
    if(shouldQuit){
      cancel(mayInterrupt);
    }
    return shouldQuit;
  }

  /**
   * Executes a Runnable instance's #run method on the main thread.
   * @param runnable The Runnable instance whose #run method should be invoked on the main thread.
   */
  public void runOnUiThread(Runnable runnable) {
    getHandler().post(runnable);
  }

  /**
   * Creates a reference to the current thread, sets that thread's priority, and initiates the
   * #performWorkInBackgroundThread method.
   *
   * Unlike most Runnable implementations, this method should not be commonly overridden.  It is not
   * marked as final in case a subclasses wants to hook into this process, but in almost all cases the
   * subclass should do its work in #performWorkInBackgroundThread rather than #run.
   */
  public void run() {
    mThread = Thread.currentThread();
    Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
    performWorkInBackgroundThread();
  }

  /**
   * Passes this instance to the common ThreadPoolExecutor, which will provide a worker thread and call this
   * instance's #run method.
   */
  public void start() {
    getThreadPoolExecutor().execute(this);
  }

  /**
   * Subclasses should override this method to perform work in the background thread provided by this class when
   * #start is called.
   *
   * Any time work needs to be published to the main thread from within the method body, use #runOnUiThread.
   *
   * Work within this method should tend to be non-atomic and test for #quitIfCancelledOrInterrupted as often as
   * possible, returning immediately if that method returns true.
   */
  public abstract void performWorkInBackgroundThread();

}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

1) best practice lazy instantiation
A better lazy field instantiation pattern is the following: https://en.wikipedia.org/wiki/Double-checked_locking

Example:

static volatile ThreadPoolExecutor helper;
public ThreadPoolExecutor getHelper() {
    ThreadPoolExecutor result = sThreadPoolExecutor;

    if (result == null) {
        return result;
    }

    synchronized(this) {
        result = helper;
        if (result == null) {
            sThreadPoolExecutor = result = new ThreadPoolExecutor(...);
        }
    }

    return result;
}

The same pattern should be used for getHandler.

Or save yourself the trouble and just always create the ThreadPoolExecutor and Handler.

2) mCancelled
Are you sure you need the whole workflow surrounding mCancelled? Relying on isInterrupted might be enough.

3) runOnUiThread
Why is runOnUiThread an instance method? It might as well be static.

4) one line creation & storing
You could let your start method return this; to do the following:

AsynchronousOperation x = new AsynchronousOperation(){}.start();  
x.cancel(true);

Personally I hate that I have to put Java Thread creation and thread starting on two different lines!

\$\endgroup\$
3
  • \$\begingroup\$ 1. gtk, will do, thank you 3. same with getHandler and getThreadPoolExecutor, they reference statics, but are instance methods - i'm not sure there's any downside to this - LMK if there is. 4. yes, or i could have it autostart - maybe a boolean in the constructor to autostart or not... great feedback, tyvm \$\endgroup\$
    – mdd-sbo
    May 18, 2016 at 13:36
  • \$\begingroup\$ Whether to make thinks static or not and how to let static and non static methods and fields interact is something people love to argue about. If you expect inheritance, overwriting or the use of different implementations don't use static. \$\endgroup\$
    – ASA
    May 21, 2016 at 9:51
  • \$\begingroup\$ In your case you have to create an instance of AsynchronousOperation, including a performWorkInBackgroundThread implementation, that might be left empty, just to call runOnUiThread. That's weird. \$\endgroup\$
    – ASA
    May 21, 2016 at 9:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.