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

ServiceBroker.DisposeAsync can lead to a deadlock #504

Closed
adrianvmsft opened this issue Jul 15, 2022 · 1 comment
Closed

ServiceBroker.DisposeAsync can lead to a deadlock #504

adrianvmsft opened this issue Jul 15, 2022 · 1 comment
Labels

Comments

@adrianvmsft
Copy link

adrianvmsft commented Jul 15, 2022

This issue was encountered by internal automation - when running 2 tests using ServiceBroker in a row causes a deadlock when each of the tests calls ServiceBroker.DisposeAsync at the end.

1ff00bffb68 <0> Nerdbank.Streams.MultiplexingStream+V1Formatter+<DisposeAsync>d__9 @ 7ff9840086c0
..1ff00bffc78 <1> Nerdbank.Streams.MultiplexingStream+<DisposeAsync>d__55 @ 7ff984006d50
..1ff00bffd88 <0> Microsoft.ServiceHub.Framework.RemoteServiceBroker+<DisposeAsync>d__34 @ 7ff9840018a0
..1ff008ae440 <2> vside.Project.QueryTests.QueryBasicTests+<BasicQueryAsync>d__4 @ 7ff983915020
..1ff008ae400 <0> vside.Project.QueryTests.QueryBasicTests+<BasicQueryTest_NetCore_Async>d__2 @ 7ff984017f20

1ff00b0f390 <0> Nerdbank.Streams.MultiplexingStream+<ReadToFillAsync>d__57 @ 7ff983abdaf0
..1ff00b0f4f0 <1> Nerdbank.Streams.MultiplexingStream+V1Formatter+<ReadFrameAsync>d__13 @ 7ff983abca00
..1ff00ac4c68 <0> Nerdbank.Streams.MultiplexingStream+<ReadStreamAsync>d__60 @ 7ff983abae40

The first async task waits on the work reported by the 2nd async task to be completed first. That never happens, despite the fact the cancellation token that is supposed to cancel is triggered.

The root cause is that the CancellationToken in Stream.ReadAsync is ignored:
dotnet/runtime#24093
dotnet/runtime#23638
dotnet/runtime#31390

@AArnott
Copy link
Collaborator

AArnott commented Jul 22, 2022

There is no general and safe way to force cancellation of I/O that underlies a Stream-derived class as a consumer of Stream. They may or may not honor the CancellationToken. Disposing the Stream can sometimes force a Read call to return 0, but it also risks thread-safety issues to do so because Stream isn't expected to be thread-safe.

What we theoretically could do here is do a runtime type check and if the stream is a named pipe stream, we could wrap it in our own stream that implements the I/O ourselves in a cancelable way by basically copying code from dotnet/runtime#72503 and dotnet/runtime#72612. This seems extremely heavy though. And since the repro for this is only in a VS test, which has already worked around the problem, I'm going to close this as Won't Fix. It will work properly on .NET 7 where the named pipe class now has cancelable I/O.

@AArnott AArnott closed this as not planned Won't fix, can't repro, duplicate, stale Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants