Cancelling tasks: Thread.interrupt() fragility

Today I reviewed some code in blueMarine as part of a moderate refactoring aimed at solving some problems with integration tests. This code is the portion that is triggered by a user selection (any thing of selection, it's an abstract class with polymorphic behaviour in implementations), scans the database in compliance with the user selection and provides the result to show in the viewer. It's the typical long task, so it uses multi-threading to avoid blocking the UI. Since the user might decide to go to another selection before the previous scan is over, a cancellation facility is needed to stop wasting CPU.

This is the typical task for the concurrency API:

AtomicReference<Future<?>> runningTask = new AtomicReference<Future<?>>();

...

Future<?> t = runningTask.getAndSet(null);

if (t != null)
  {
    t.cancel(true);
  }

ExecutorService executorService = Executors.newFixedThreadPool(4);
runningTask.set(executorService.submit(new Runnable()
  {
     public void run()
       {
          while (...)
            {
               ...
               if (thread.isInterrupted()) // this indeed is performed by another called method in a different class
                 {
                   break;
                 }
            }

          runningTask.set(null);
       }
  }));

The key of everything is the Future.cancel(true) method that will call Thread.interrupt() on the related thread. So, apparently the only thing that we should do is periodically poll for the isInterrupted() method and eventually stop our computation. Often this is performed by throwing an InterruptedException.

Now, there are two ways for querying a thread to see whether is interrupted; the latter way is to call Thread.interrupted(). This version will reset the interrupted flag; in other words, any further probe for the interruption will return false. If an exception is thrown, the catch code is given the chance to set the flag back to true.

So, it boils down to not using interrupted() in favour of isInterrupted(), right? Well, today's tests demonstrated that I was able to cancel a previous running task only once out of a dozen runs (and probably less). I can only think that my code calls some library (NetBeans stuff in this case) that potentially calls interrupted() without throwing an InterruptedException. So my code never understands that the thread is running in has been interrupted.

I'm currently investigating where the problem is - but, indeed, I came to the conclusion that the Thread.interrupt() mechanism is too fragile. This section is a part of blueMarine that is likely to be expanded (for different kinds of queries) and there's potentially a lot of code that could be called in that thread, potentially by third parties. Having the code to respect the Thread.interrupted() stuff seems too a heavy prerequisite for the contract.

So I decided to find a different, reliable and controllable solution. Of course, what I need is just a boolean flag to tell the code that the processing should be interrupted; it's just a matter of decoupling. The above code doesn't know which class will implement the processing method and vice versa. So I've ended up with a solution based on ThreadLocal:

    public final class CancellationController
      {
        private static final ThreadLocal INSTANCE = new ThreadLocal()
          {
            @Override
            protected CancellationController initialValue()
              {
                return new CancellationController(Thread.currentThread());
              }
          };

        private volatile boolean cancelled = false;

        @Nonnull
        private final Thread thread;

        private CancellationController (@Nonnull final Thread thread)
          {
            this.thread = thread;
          }

        public static boolean isCancelled()
          {
            return INSTANCE.get().cancelled;
          }

        @Override
        @Nonnull
        public String toString()
          {
            return String.format("CancellationController[%s]", thread.getName());
          }

        protected void cancel()
          {
            thread.interrupt();
            cancelled = true;
          }

        @Nonnull
        protected static CancellationController get()
          {
            return INSTANCE.get();
          }

        protected void reset()
          {
            cancelled = false;
          }
      }
     


The original code now is:

AtomicReference<CancellationController> runningTask = new AtomicReference<CancellationController>();

...

CancellationController t = runningTask.getAndSet(null);

if (t != null)
  {
    t.cancel();
  }

ExecutorService executorService = Executors.newFixedThreadPool(4);
executorService.submit(new Runnable())
  {
     public void run()
       {
          CancellationController.get().reset();
          runningTask.set(CancellationController.get());

          while (...)
            {
               ...
               if (CancellationController.get().isCancelled())
                 {
                   break;
                 }
            }

          runningTask.set(null);
       }
  });

I suspect it's a recurrent problem so I'd like to know whether others are doing the same, or whether there are simpler solutions. Thanks.