-
Bug
-
Resolution: Fixed
-
P3
-
5.0
-
b49
-
generic
-
generic
Tim Peierls writes:
I'm wondering about this code in FutureTask.Sync:
boolean innerCancel(boolean mayInterruptIfRunning) {
int s = getState();
if (ranOrCancelled(s) || !compareAndSetState(s, CANCELLED))
return false;
if (mayInterruptIfRunning) {
Thread r = runner;
if (r != null)
r.interrupt();
}
releaseShared(0);
done();
return true;
}
If the Sync transitions between initial state and RUNNING
between the calls to getState and compareAndSetState, the
CAS will fail, and the call to Future.cancel() will return
false. Can that be right?
--tim
David Holmes writes:
I don't think so. If the task becomes RUNNING then we should cancel it. If
the CAS fails we need to retry compareAndSetState(RUNNING, CANCELLED) and if
that fails then we return false.
David
Doug Lea writes:
No, this must be changed to retry. WHich I just did. For symmetry, I
also changed innerSet and innerSetException in the same way. Even
though a task should not be transitioning in either of those cases, we
don't say anything precluding it, so it seems best to do the same
thing.
Thanks very much!! Our first j.u.c. functionality bug. Please keep
looking for more!
There is a race window during which a starting task incorrectly
cannot be cancelled.
There's no test case.
If necessary, I probably could make a test case, but the likelihood
of it triggering the bug in any finite test time would be small. On
the other hand, the probablity of this hitting someone sometime is
high, so it must be fixed.
I'm wondering about this code in FutureTask.Sync:
boolean innerCancel(boolean mayInterruptIfRunning) {
int s = getState();
if (ranOrCancelled(s) || !compareAndSetState(s, CANCELLED))
return false;
if (mayInterruptIfRunning) {
Thread r = runner;
if (r != null)
r.interrupt();
}
releaseShared(0);
done();
return true;
}
If the Sync transitions between initial state and RUNNING
between the calls to getState and compareAndSetState, the
CAS will fail, and the call to Future.cancel() will return
false. Can that be right?
--tim
David Holmes writes:
I don't think so. If the task becomes RUNNING then we should cancel it. If
the CAS fails we need to retry compareAndSetState(RUNNING, CANCELLED) and if
that fails then we return false.
David
Doug Lea writes:
No, this must be changed to retry. WHich I just did. For symmetry, I
also changed innerSet and innerSetException in the same way. Even
though a task should not be transitioning in either of those cases, we
don't say anything precluding it, so it seems best to do the same
thing.
Thanks very much!! Our first j.u.c. functionality bug. Please keep
looking for more!
There is a race window during which a starting task incorrectly
cannot be cancelled.
There's no test case.
If necessary, I probably could make a test case, but the likelihood
of it triggering the bug in any finite test time would be small. On
the other hand, the probablity of this hitting someone sometime is
high, so it must be fixed.