Fix corner-case handling of cancellation exception in ForEachAsync #59065
Conversation
If code in Parallel.ForEachAsync throws OperationCanceledExceptions containing the CancellationToken passed to the iteration and that token has _not_ had cancellation requested (so why are they throwing with it) and there are no other exceptions, the ForEachAsync will effectively hang after failing to complete the task returned from it. The issue stems from how we treat cancellation. If the user-supplied token hasn't been canceled but we have OperationCanceledExceptions for the token passed into the iteration (the "internal" token), it can only have been canceled because an exception occurred. We filter out these cancellation exceptions, leaving just the exceptions that are deemed to have caused the failure in the first place. But the code doesn't currently account for the possibility that the developer is (arguably erroneously) throwing such an OperationCanceledException with the internal cancellation token as that root failure. The fix is to only filter out these OCEs if there are other exceptions besides them.
|
Tagging subscribers to this area: @dotnet/area-system-threading-tasks Issue DetailsIf code in Parallel.ForEachAsync throws OperationCanceledExceptions containing the CancellationToken passed to the iteration and that token has not had cancellation requested (so why are they throwing with it) and there are no other exceptions, the ForEachAsync will effectively hang after failing to complete the task returned from it. The issue stems from how we treat cancellation. If the user-supplied token hasn't been canceled but we have OperationCanceledExceptions for the token passed into the iteration (the "internal" token), it can only have been canceled because an exception occurred. We filter out these cancellation exceptions, leaving just the exceptions that are deemed to have caused the failure in the first place. But the code doesn't currently account for the possibility that the developer is (arguably erroneously) throwing such an OperationCanceledException with the internal cancellation token as that root failure. The fix is to only filter out these OCEs if there are other exceptions besides them. Fixes #58981
|
...ibraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/Parallel.ForEachAsync.cs
Outdated
Show resolved
Hide resolved
|
I've reconsidered this, and I think it does make sense to just stop any such special casing / filtering. That's simpler and cheaper and explainable and consistent, and in the 99.9% case, it won't matter anyway as awaiting the returned task will only propagate the first exception, anyway. |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1243826574 |
|
/backport to release/6.0-rc2 |
|
Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1243886472 |
ghost
locked as resolved and limited conversation to collaborators
3 participants
If code in Parallel.ForEachAsync throws OperationCanceledExceptions containing the CancellationToken passed to the iteration and that token has not had cancellation requested (so why are they throwing with it) and there are no other exceptions, the ForEachAsync will effectively hang after failing to complete the task returned from it.
The issue stems from how we treat cancellation. If the user-supplied token hasn't been canceled but we have OperationCanceledExceptions for the token passed into the iteration (the "internal" token), it can only have been canceled because an exception occurred. We filter out these cancellation exceptions, leaving just the exceptions that are deemed to have caused the failure in the first place. But the code doesn't currently account for the possibility that the developer is (arguably erroneously) throwing such an OperationCanceledException with the internal cancellation token as that root failure. The fix is to only filter out these OCEs if there are other exceptions besides them.
Fixes #58981