You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think we've found another cancellation corner case in CoroutineScope.future {}.
In a race between asynchronous cancellation from Future.cancel() and cancellation from failure of the executing Job, ListenableFutureCoroutine.onCancelled() may execute after the Future has already been completed as cancelled.
When onCancelled() runs after the Future is complete, it invokes the CoroutineExceptionHandler and escalates the failure:
override fun onCancelled(cause: Throwable, handled: Boolean) {
if (!future.completeExceptionallyOrCancel(cause) && !handled) {
// prevents loss of exception that was not handled by parent & could not be set to JobListenableFuture
handleCoroutineException(context, cause)
}
}
This is different from how FutureTask treats failure after cancellation.
I haven't minimized a test case, but my reading is:
Future.cancel() is called on the returned Future
The cancelling thread of control executes JobListenableFuture.cancel(), succeeds in cancelling the Future, then calls jobToCancel.cancel()
Concurrently, the thread of control executing a continuation in that Job completes in failure, which also calls cancel() on that same Job
If the thread of control executing the continuation wins the race, onCancelled() gets called with a failure, fails to complete the already-complete-as-cancelled Future with that failure, and so invokes the CoroutineExceptionHandler
In contrast, FutureTask uses CAS from NEW -> COMPLETING when the submitted function finishes for any reason. If that CAS fails, the completion is dropped, whether or not the completion is a failure. Cancellation makes the same CAS away from NEW, and there's no else case if completion of the running function with an Exception doesn't win the race.
Since future {} calls the CoroutineExceptionHandler, errors that would be ignored in a function run with an Executor instead execute a failure handler. So at the moment, under cancellation, futures from myCoroutineScope.future(::myFunc) are incompatible with those returned from myExecutor.submit(::myFunc).
I think to maintain compatibility, future {} would have to attempt to complete its Future, but return from the stack and drop the completion if the future is already complete, for any reason?
Incidentally, I think the documentation on ListenableFutureCoroutine is out of date. That class is only used by future {}, but the docstring refers to its use in asListenableFuture {}.
The text was updated successfully, but these errors were encountered:
…oroutineExceptionHandler.
See Kotlin#2774 and Kotlin#2791 for more context.
This change makes `future` coroutine builder consistent with `java.util.concurrent.FutureTask` which also drops exceptions that happens after successful cancellation.
…oroutineExceptionHandler (Kotlin#2840)
This change makes `future` coroutine builder consistent with `java.util.concurrent.FutureTask` which also drops exceptions that happen after successful cancellation.
FixesKotlin#2774FixesKotlin#2791
…oroutineExceptionHandler (Kotlin#2840)
This change makes `future` coroutine builder consistent with `java.util.concurrent.FutureTask` which also drops exceptions that happen after successful cancellation.
FixesKotlin#2774FixesKotlin#2791
I think we've found another cancellation corner case in
CoroutineScope.future {}
.In a race between asynchronous cancellation from
Future.cancel()
and cancellation from failure of the executingJob
,ListenableFutureCoroutine.onCancelled()
may execute after theFuture
has already been completed as cancelled.When
onCancelled()
runs after theFuture
is complete, it invokes theCoroutineExceptionHandler
and escalates the failure:This is different from how
FutureTask
treats failure after cancellation.I haven't minimized a test case, but my reading is:
Future.cancel()
is called on the returnedFuture
JobListenableFuture.cancel()
, succeeds in cancelling theFuture
, then callsjobToCancel.cancel()
Job
completes in failure, which also callscancel()
on that sameJob
onCancelled()
gets called with a failure, fails to complete the already-complete-as-cancelledFuture
with that failure, and so invokes theCoroutineExceptionHandler
In contrast,
FutureTask
uses CAS from NEW -> COMPLETING when the submitted function finishes for any reason. If that CAS fails, the completion is dropped, whether or not the completion is a failure. Cancellation makes the same CAS away fromNEW
, and there's noelse
case if completion of the running function with anException
doesn't win the race.Since
future {}
calls theCoroutineExceptionHandler
, errors that would be ignored in a function run with anExecutor
instead execute a failure handler. So at the moment, under cancellation, futures frommyCoroutineScope.future(::myFunc)
are incompatible with those returned frommyExecutor.submit(::myFunc)
.I think to maintain compatibility,
future {}
would have to attempt to complete itsFuture
, but return from the stack and drop the completion if the future is already complete, for any reason?Incidentally, I think the documentation on
ListenableFutureCoroutine
is out of date. That class is only used byfuture {}
, but the docstring refers to its use inasListenableFuture {}
.The text was updated successfully, but these errors were encountered: