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 async read issue with anonymous PipeStream #4

Merged
merged 2 commits into from
Feb 15, 2022
Merged

Fix async read issue with anonymous PipeStream #4

merged 2 commits into from
Feb 15, 2022

Conversation

xoofx
Copy link
Contributor

@xoofx xoofx commented Feb 15, 2022

Hey,
I encountered a bug that took me quite some time to find out.
I was launching multiple msbuild concurrently (e.g 20) on separated threads from the same process and using Anonymous pipes was making the application taking 40s to complete while the same operation should take no more than e.g 1s in total.
It turns out that PipeBuffer was using ReadAsync on a pipe stream which seems to be not supported actually, hence the weird behaviors.

For more details:

I still keep around the cancellationToken.IsCancellationRequested as FillFromStream is called in a loop in the end, so still worth to have it around.

@daveaglick
Copy link
Owner

Nice detective work! It's interesting that the docs for AnonymousPipeClientStream don't actually mention this limitation. In fact, it's worse because those docs make it sound like it's actually supported:

Exposes the client side of an anonymous pipe stream, which supports both synchronous and asynchronous read and write operations.

I suppose there might be a platform difference here - non-Windows platforms where the underlying anonymous pipe is implemented differently may not exhibit the same limitations. Rather than speculate or add OS checks, your solution seems simplest and safest though.

I'm going to follow-up with a documentation issue on those AnonymousPipeClientStream docs too - even though it's niche, seems like a good thing to have called out.

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

Successfully merging this pull request may close these issues.

2 participants