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

Investigate having custom SynchronizationContext that handles async void #4709

Open
Youssef1313 opened this issue Jan 18, 2025 · 5 comments
Open

Comments

@Youssef1313
Copy link
Member

Youssef1313 commented Jan 18, 2025

Seems reasonable I'd guess. I'd find the behavior more intuitive if it behaved like xunit does: xunit registers its own SynchronizationContext in order to make async void work, both in terms of knowing when the test completes and knowing when an exception occurs. async void still isn't recommended, but having one doesn't put the whole process in jeopardy.

Originally posted by @stephentoub in dotnet/docs#44419 (comment)

@stephentoub Would you expect the custom synchronization context to also "preserve" thread apartment state after an await? Currently, if we have a test method marked as STA, I think the code before the first await will be STA, but the code after may not because it could end up on a ThreadPool thread. Or do you think it's fine to end up non-STA after an await?

cc @Evangelink @MarcoRossignoli As we have had previous discussions around that area.

@Youssef1313
Copy link
Member Author

@stephentoub FYI, xUnit v3 removed their AsyncTestSyncContext and are now disallowing async void test methods completely. An analyzer was implemented with Error severity in xUnit 3 for async void. https://xunit.net/xunit.analyzers/rules/xUnit1049

@Evangelink
Copy link
Member

We didn't go toward having any rule as error by default but maybe it does makes sense

@Youssef1313
Copy link
Member Author

@Evangelink The original suggestion is a bit different though.

@stephentoub
Copy link
Member

stephentoub commented Jan 20, 2025

I'm fine if it's simply blocked by default. What I find problematic is the middle ground where it's not blocked but doesn't actually work.

@Youssef1313
Copy link
Member Author

async void test methods are not supported in our case (and in case of xUnit 3 as well). But that "middle ground" still exists if the test method is simply void (not async) and you call some other async void method in it. That part is no longer handled in xUnit 3 (can end up with a silent failure). For our case, we will produce an analyzer warning.

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

No branches or pull requests

3 participants