0 Replies Latest reply: Apr 19, 2012 8:54 PM by 931966 RSS

    Race condition in FutureTask.cancel()?

    931966
      Future.cancel() takes a boolean parameter specifying whether to interrupt the thread running the task. Looking at FutureTask's implementation of this method, inside FutureTask.Sync.innerCancel(), it appears that there's no guarantee that the task thread won't be interrupted until some time after completing the task.

      Consider the execution interleaving below, where statements (1), (2), (3), and (4) are run in that order. The canceling thread successfully cancels the task, at which point the task thread notices this and finishes working on the task. The task thread (as part of a worker thread pool) then goes on to run a different task. At this point the canceler thread runs (4) and interrupts the task thread in the middle of doing something else.

      Task thread:

      void innerSet(V v) {
           for (;;) {
                int s = getState(); // (2)
                if (s == RAN)
                return;
      if (s == CANCELLED) {
                // aggressively release to set runner to null,
                // in case we are racing with a cancel request
                // that will try to interrupt runner
      releaseShared(0); // (3)
      return;
      }
                if (compareAndSetState(s, RAN)) {
      result = v;
      releaseShared(0);
      done();
                return;
      }
      }
      }

      Canceling thread:

      boolean innerCancel(boolean mayInterruptIfRunning) {
           for (;;) {
                int s = getState();
                if (ranOrCancelled(s))
                return false;
                if (compareAndSetState(s, CANCELLED))
                break;
           }
      if (mayInterruptIfRunning) {
      Thread r = runner; // (1)
      if (r != null)
      r.interrupt(); // (4)
      }
      releaseShared(0);
      done();
      return true;
      }


      I've simulated this situation occurring by inserting breakpoints in test code. The documentation doesn't mention when the interrupt gets delivered, but this behavior seems like it would make task cancellation very dangerous and a likely source of bugs, since in general threads don't expect to have to handle spurious interrupts. The problem appears to be that there's no synchronization barrier establishing a happens-before relationship between the cancellation thread calling Thread.interrupt() and the task thread completing the task and moving on. I'm wondering if this behavior is intended, in which case I think it would bear mention in the documentation, or if it's a genuine bug. I think that avoiding this bug would necessitate adding an additional synchronization state, CANCELING, to the FutureTask.Sync class, then changing the logic as follows:

      innerCancel(): compareAndSetState(s, CANCELING) -> r.interrupt() -> state = CANCELLED

      innerSet(), innerSetException():
      if (s == CANCELING) {
      aquireShared(0);
      return;
      }

      In addition, the interrupted status would probably have to be cleared at some point.