Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UnobservedTaskException from SemaphoreSlim.WaitAsync #60890

Merged
merged 2 commits into from
Nov 5, 2021

Conversation

stephentoub
Copy link
Member

If a SemaphoreSlim.WaitAsync times out, it correctly returns false, but it also results in TaskScheduler.UnobservedTaskException being raised unexpectedly, due to internal use of a faulted task whose exception isn't observed. This fixes that by marking any such exceptions as having been observed.

Fixes #60856

@ghost
Copy link

ghost commented Oct 26, 2021

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

If a SemaphoreSlim.WaitAsync times out, it correctly returns false, but it also results in TaskScheduler.UnobservedTaskException being raised unexpectedly, due to internal use of a faulted task whose exception isn't observed. This fixes that by marking any such exceptions as having been observed.

Fixes #60856

Author: stephentoub
Assignees: -
Labels:

area-System.Threading.Tasks

Milestone: 7.0.0

If a SemaphoreSlim.WaitAsync times out, it correctly returns false, but it also results in TaskScheduler.UnobservedTaskException being raised unexpectedly, due to internal use of a faulted task whose exception isn't observed.  This fixes that by marking any such exceptions as having been observed.
@stephentoub stephentoub merged commit fd07214 into dotnet:main Nov 5, 2021
@stephentoub stephentoub deleted the fixsemexception branch November 5, 2021 15:06
@adamsitnik
Copy link
Member

@stephentoub should we backport it to 6.0.1?

@stephentoub
Copy link
Member Author

It'd be fine to propose doing so if you think it's worthwhile.

@adamsitnik
Copy link
Member

It'd be fine to propose doing so if you think it's worthwhile.

I think that it's worthwhile. It's quite common to use TaskScheduler.UnobservedTaskException and it should be safe to assume that the event is raised for unobserved exceptions. And if exceptions are not used for flow control , it means that there is an actual problem when the even is raised . So if there is not too much risk in the fix, we should consider backporting it.

@stephentoub
Copy link
Member Author

So if there is not too much risk in the fix, we should consider backporting it.

It should be low risk; it's just setting a flag that says "treat this as already observed". We could let it bake in main for a bit first if desired, or not.

@stephentoub
Copy link
Member Author

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1451429820

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SemaphoreSlim.WaitAsync() begin fire TimeoutException on TaskScheduler.UnobservedTaskException in net6.0?
2 participants