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

Consider implementing AnonymousPipeServer/ClientStream on Unix on socketpair #44329

Closed
stephentoub opened this issue Nov 5, 2020 · 5 comments
Labels
area-System.IO tenet-performance Performance related issue tenet-reliability Reliability/stability related issue (stress, load problems, etc.)
Milestone

Comments

@stephentoub
Copy link
Member

Currently AnonymousPipeServerStream and AnonymousPipeClientStream on Unix are implemented on top of pipe. This makes sense, but it also makes it difficult for us with our current infrastructure to properly implement async I/O for these streams. This means that Read/WriteAsync on these types are actually async-over-sync and also that cancellation only ends up doing an up-front check and doesn't actually support canceling an operation in progress.

NamedPipeServerStream and NamedPipeClientStream on Unix are instead implemented on top of unix domain sockets via System.Net.Socket, and that implementation makes it possible to just rely on the built-in async and cancellation support Socket provides. We could do the same thing with anonymous pipes, using socketpair instead of pipe to create the underlying file descriptors, and then wrapping those with Socket just as we do in the named pipes.

@tmds, thoughts on this? There would likely be some subtle differences in behavior, especially around buffering. I'm wondering as well if it might be visible in a bad way for anyone extracting the file descriptor from the SafeHandle and using it in some way that pipe-generated file descriptors would work but socketpair-generated file descriptors wouldn't.

@stephentoub stephentoub added area-System.IO tenet-performance Performance related issue tenet-reliability Reliability/stability related issue (stress, load problems, etc.) labels Nov 5, 2020
@stephentoub stephentoub added this to the 6.0.0 milestone Nov 5, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 5, 2020
@tmds
Copy link
Member

tmds commented Nov 6, 2020

If the AnonymousPipeClientStream is constructed from a handle that is a pipe (because the peer is not a .NET 6+ app) it will give issues when we treat it as a Socket. Socket specific APIs will return ENOTSOCK.

We could try make Socket more pipe friendly by using calls like read/write instead of recvmsg/sendmsg. Then the handle can remain a pipe and we can still leverage the SocketAsyncEngine.

@stephentoub
Copy link
Member Author

If the AnonymousPipeClientStream is constructed from a handle that is a pipe (because the peer is not a .NET 6+ app) it will give issues when we treat it as a Socket. Socket specific APIs will return ENOTSOCK.

Good point. We could allow APCS to support both; you just wouldn't get the benefits in the case where it was pipe.

We could try make Socket more pipe friendly by using calls like read/write instead of recvmsg/sendmsg.

Would that have any negative throughput implications for Socket?

Then the handle can remain a pipe and we can still leverage the SocketAsyncEngine.

I don't want multiple instances of the SocketAsyncEngine in play, so we would need to expose it in some way for the other assembly to use. This gets back then to how do we push that infrastructure lower for any assembly to use. If you have ideas for what that looks like, I'd love to hear them. We've talked at a high-level about building this support into ThreadPool, and having some kind of API on ThreadPool that sits on top of this. If you have the time and inclination, that could be really interesting to experiment with and come up with a proposal for. Our async I/O support on unix outside of Socket today is lacking.

@tmds
Copy link
Member

tmds commented May 4, 2021

With #44647 AnonymousPipeServer/ClientStream use SocketAsyncEngine. This issue may be closed.

@manandre
Copy link
Contributor

manandre commented May 16, 2021

@tmds With #44647 NamedPipeServer/ClientStream seem to also use SocketAsyncEngine. Can the related issue #44331 be closed?

@stephentoub
Copy link
Member Author

Yup.

@ghost ghost locked as resolved and limited conversation to collaborators Jun 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO tenet-performance Performance related issue tenet-reliability Reliability/stability related issue (stress, load problems, etc.)
Projects
None yet
Development

No branches or pull requests

5 participants