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

Make NetworkStream compatible with non-blocking I/O #22506

Closed
roji opened this issue Jun 26, 2017 · 40 comments
Closed

Make NetworkStream compatible with non-blocking I/O #22506

roji opened this issue Jun 26, 2017 · 40 comments
Labels
area-System.Net design-discussion Ongoing discussion about design without consensus enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@roji
Copy link
Member

roji commented Jun 26, 2017

The predominant way to perform I/O is async. However, in some scenarios non-blocking I/O (or polling) is better adapted - the ability to read or write as much as possible, along with a guarantee that the call would not block. This allows retaining a very deterministic, single-thread model where I/O occurs opportunistically where the application chooses, rather than launching an asynchronous operation that may occur in parallel with other processes, require synchronization etc. Note that non-blocking is quite a common pattern in Unix (but not in Windows, which may explain the lack of support in .NET).

Non-blocking I/O is currently supported at the Socket level by setting Blocking to true, and then calling Send and Receive. However, trying to construct a NetworkStream over a non-blocking socket throws an IOException (The operation is not allowed on a non-blocking Socket).

Note that .NET's Stream.Write() returns void (unlike the Linux write system call). This probably means that the introduction of a new API element would be required to support non-blocking writing.

@svick
Copy link
Contributor

svick commented Jun 26, 2017

This allows retaining a very deterministic, single-thread model where I/O occurs opportunistically where the application chooses, rather than launching an asynchronous operation that may occur in parallel with other processes, require synchronization etc.

Couldn't you achieve something very similar using a single-threaded synchronization context?

@roji
Copy link
Member Author

roji commented Jun 26, 2017

Maybe you can, in some scenarios - I actually have something like that in Npgsql - but depending on what you're trying to do it can be very convoluted and unwieldy. At least in the Unix world, non-blocking I/O is a proven, simple and lightweight programming model that happens to be poorly supported by .NET APIs.

@davidfowl
Copy link
Member

WriteAsync and ReadAsync support non blocking IO just fine, the only thing we're really discussion here in a API pattern difference, basically the reactor vs proactor pattern. All async IO in .NET is via the proactor pattern and the granularity is the operation itself (write or read).

It seems like you're asking for a reactor pattern to be exposed (which is more like java's NIO implementation). That would require exposing an event loop of some sort and it would be an entirely different paradigm to what .NET developers are used to. Not only that but it's not exactly trivial to implement windows IOCP on top of that pattern. This is why java's NIO2 added support for the proactor pattern and async io (so that IOCP support could be implemented much easier). It's also the same reason why libuv adapts the event loop into a per operation callback model.

@roji
Copy link
Member Author

roji commented Jul 2, 2017

It seems like we've had this discussion at least once before... Async I/O and non-blocking I/O really are two different things - the former is about attaching a callback to the completion, which in many cases will execute on another thread, while the latter is simply about reading/writing as much as possible, right now, without ever yielding the thread (we can call this polling). It's not about proactor or reactor - non-blocking I/O on Unix is a socket option that alters the behavior of the read and write system calls; it's this behavior that I'd like to have .NET support better. There may be a language issue here - when I'm saying non-blocking I/O, I'm referring to the traditional Unix way of doing I/O without blocking.

Now, if anyone wants to claim that Unix-style non-blocking I/O is useless in 2017 or has been made totally obsolete by async I/O, that's another discussion; but before we even get into that it's important for everyone to understand that there are two different I/O models here.

@roji
Copy link
Member Author

roji commented Jul 2, 2017

Maybe the thing to make this clear, is that the Unix-style non-blocking I/O I'm talking about can be called non-blocking synchronous I/O, as distinct from asynchronous I/O, which is non-blocking by definition.

@davidfowl
Copy link
Member

It's not about proactor or reactor - non-blocking I/O on Unix is a socket option that alters the behavior of the read and write system calls; it's this behavior that I'd like to have .NET support better.

You're exactly talking about those 2 patterns though. What happens when the I/O operation needs to block (i.e. when you get an EWOULDBLOCK from the non blocking socket operation). What do you poll to know if you can write/read again?

If you squint, you can imagine socket.Read/WriteAsync just completing synchronously while the operation wouldn't block and when it would, internally the socket would register for another notification on the event loop and call you back when it's ready. This could all be done with async await without exposing the underlying I/O model.

There may be a language issue here - when I'm saying non-blocking I/O, I'm referring to the traditional Unix way of doing I/O without blocking.

There's no language issue. This is an API question.

Maybe the thing to make this clear, is that the Unix-style non-blocking I/O I'm talking about can be called non-blocking synchronous I/O, as distinct from asynchronous I/O, which is non-blocking by definition.

Exposing APIs that directly expose non blocking socket API wouldn't work at well (or at all really) on windows for all of the reasons I mentioned previously.

Since you seem to be looking for something very specific, propose the APIs you need and some sample before and after code.

@roji
Copy link
Member Author

roji commented Jul 2, 2017

It's not about proactor or reactor - non-blocking I/O on Unix is a socket option that alters the behavior of the read and write system calls; it's this behavior that I'd like to have .NET support better.

You're exactly talking about those 2 patterns though. What happens when the I/O operation needs to block (i.e. when you get an EWOULDBLOCK from the non blocking socket operation). What do you poll to know if you can write/read again?

There may a misunderstanding on my side here with regards to proactor/reactor.

The classical Unix model would be to use select (exposed in .NET), or epoll, to wait until a socket is ready, and then non-blocking reading to exhaust data without blocking. When you get EWOULDBLOCK you go back to select. .NET currently doesn't support doing the second part very well. Again, you may criticize this programming model - that's fine - but I'm trying to show it's a different model before we go into that.

In other scenarios, when you get EWOULDBLOCK you go and do something else, and opportunistically retry later. This allows you to interleave non-blocking I/O operations with other operations, performing I/O when possible. I have a case like that in Npgsql (PostgreSQL .NET driver) which is what prompted this discussion, although it's a bit out of scope.

If you squint, you can imagine socket.Read/WriteAsync just completing synchronously while the operation wouldn't block and when it would, internally the socket would register for another notification on the event loop and call you back when it's ready. This could all be done with async await without exposing the underlying I/O model.

I get that, but there's one crucial difference - you're forced to yield the executing thread and have the continuation execute somewhere else. That's exactly the power of async - improve scalability by not blocking the thread - but that's exactly the detail in which it differs from classic non-blocking I/O. So almost by definition, it doesn't seem correct to say "you can do all that with async/await", whose sole purpose is to yield the thread.

Since you seem to be looking for something very specific, propose the APIs you need and some sample before and after code.

It's really very simple. The synchronous NetworkStream.{Read,Write} methods can be seem as logical .NET counterparts to the Unix read/write system calls - the API is in fact nearly identical. When the socket is set to non-blocking mode (already supported in .NET), Unix read/write's behavior change, and the methods return immediately. I'd like the same behavior to be possible with NetworkStream.{Read,Write}.

As I wrote before, one API problem is that .NET's Stream.Write returns void rather than int, unlike Unix write - this makes it problematic for it to work in non-blocking (it can't return how much it managed to write). That's quite an unfortunate design decision.

It's OK for you guys to say no to this feature because:

  • Windows doesn't support Unix-style non-blocking I/O
  • The Stream.Write() method returns void, you don't want to add another method just for non-blocking and you can't see another way.
  • You consider Unix-style non-blocking to be obsoleted by async (although that's a discussion to be had)

What I'm looking for before that is more of an acknowledgement/recognition that this I/O programming model isn't already supported in .NET if I just squint the right way.

@davidfowl
Copy link
Member

The classical Unix model would be to use select (exposed in .NET), or epoll, to wait until a socket is ready, and then non-blocking reading to exhaust data without blocking. When you get EWOULDBLOCK you go back to select. .NET currently doesn't support doing the second part very well. Again, you may criticize this programming model - that's fine - but I'm trying to show it's a different model before we go into that.

It's literally exactly the 2 patterns I mentioned. You want the reactor model. You're basically asking for a .NET API that literally exposes epoll/kqueue.

As I wrote before, one API problem is that .NET's Stream.Write returns void rather than int, unlike Unix write - this makes it problematic for it to work in non-blocking (it can't return how much it managed to write). That's quite an unfortunate design decision.

I don't think that's a problem, in fact it's correct. Stream is not an IO abstraction, it's an abstraction over streaming data, which can be from non IO sources. It doesn't belong on Stream, it belongs on socket.

The Stream.Write() method returns void, you don't want to add another method just for non-blocking and you can't see another way.

It wouldn't be on Stream, it would be NetworkStream or maybe just Socket.

IMO The .NET way to expose this would be a TryRead/TryWrite on the appropriate objects (NetworkStream and Socket). If the underlying polling implementation wasn't exposed, that would need to happen automatically on your behalf and you would have no way of knowing when you could read/write again (other than calling TryRead/TryWrite in a loop).

The issue with using ReadAsync and WriteAsync is that when you do encounter an EWOULDBLOCK situation, there's no way to cancel that pending read or write. With pure non blocking IO, you can choose to do nothing or start polling again, or do something else (which is what you mention above). Maybe you could hack it by cancelling the operation when it went async (but it would probably be super inefficient):

while(true)
{
    var cts = new CancellationTokenSource();
    var task = networkStream.WriteAsync(..., cts.Token);
    if (task.IsCompleted)
    {
        // Synchronous write, carry on
    }
    else
    {
       // Async operation has been queue so do nothing...
       cts.Cancel();
       break;
    }
}

Alternatively:

while(networkStream.TryWrite(...))
{
}

// Failed to write synchronously, go do something else, register for notifications to know when you can write again...

@roji
Copy link
Member Author

roji commented Jul 2, 2017

The classical Unix model would be to use select (exposed in .NET), or epoll, to wait until a socket is ready, and then non-blocking reading to exhaust data without blocking. When you get EWOULDBLOCK you go back to select. .NET currently doesn't support doing the second part very well. Again, you may criticize this programming model - that's fine - but I'm trying to show it's a different model before we go into that.

It's literally exactly the 2 patterns I mentioned. You want the reactor model. You're basically asking for a .NET API that literally exposes epoll/kqueue.

Not really, I'm asking for a .NET API that exposes read/write (or more precisely their non-blocking support). Exposing epoll/kqueue is another question.

As I wrote before, one API problem is that .NET's Stream.Write returns void rather than int, unlike Unix write - this makes it problematic for it to work in non-blocking (it can't return how much it managed to write). That's quite an unfortunate design decision.

I don't think that's a problem, in fact it's correct. Stream is not an IO abstraction, it's an abstraction over streaming data, which can be from non IO sources. It doesn't belong on Stream, it belongs on socket.
[...]
It wouldn't be on Stream, it would be NetworkStream or maybe just Socket.

I don't see why the fact that you can stream data from non-IO sources means that your abstraction shouldn't support IO-specific features. For data sources where writing always fully completes you could simply return the number of bytes the user requested to write. I'm also not sure what your definition of non-IO sources is - Unix read/write operates on file descriptors, which can themselves represents various data sources. In other words, read/write are themselves a streaming abstraction - just that they happen to also support non-blocking IO for sources where it can occur.

Non-blocking is already supported on Socket via Send and Receive. However, these are completely outside the stream abstraction, making it impossible to can't compose over them (e.g. non-blocking support in SslStream).

IMO The .NET way to expose this would be a TryRead/TryWrite on the appropriate objects (NetworkStream and Socket).

That could be a valid API, although I'd again see it on Stream (with a default implementation calling Read/Write). Another minor issue is that Try* in .NET APIs usually means something that can succeed or not, whereas here we're talking about something that can partially, full complete or not at all. In other words, we need something that returns an int rather than a bool.

If the underlying polling implementation wasn't exposed, that would need to happen automatically on your behalf and you would have no way of knowing when you could read/write again (other than calling TryRead/TryWrite in a loop).

Knowing when you can read/write again is a concern that is totally separate from this question. Maybe you're using Socket.Select (already exposed), maybe you've wrapped epoll/kqueue, or maybe you're just trying opportunistically from time to time (in which case there's no underlying polling implementation at all).

The issue with using ReadAsync and WriteAsync is that when you do encounter an EWOULDBLOCK situation, there's no way to cancel that pending read or write. With pure non blocking IO, you can choose to do nothing or start polling again, or do something else (which is what you mention above). Maybe you could hack it by cancelling the operation when it went async (but it would probably be super inefficient):

Yep, this is what I meant by saying that async I/O and (Unix-style) non-blocking I/O are separate concepts. You can view non-blocking I/O as async I/O which magically (and very efficiently) cancels itself if it cannot complete synchronously.

@davidfowl
Copy link
Member

davidfowl commented Jul 2, 2017

I'm also not sure what your definition of non-IO sources is

DeflateStream, BufferedStream, MemoryStream... The list goes on..

Non-blocking is already supported on Socket via Send and Receive. However, these are completely outside the stream abstraction, making it impossible to can't compose over them (e.g. non-blocking support in SslStream).

Knowing when you can read/write again is a concern that is totally separate from this question. Maybe you're using Socket.Select (already exposed), maybe you've wrapped epoll/kqueue, or maybe you're just trying opportunistically from time to time (in which case there's no underlying polling implementation at all).

That seems like a 1/2 baked abstraction to me. Can you think of any example in any other framework that has a concept of a non blocking read/write that isn't directly related to IO?

@roji
Copy link
Member Author

roji commented Jul 2, 2017

I'm also not sure what your definition of non-IO sources is

DeflateStream, BufferedStream, MemoryStream... The list goes on..

I may be nit-picking here, but the first two are intermediate/layers on top of other streams, while MemoryStream can be viewed as accessing memory via an I/O abstraction. I'm not trying to be argumentative - just to say that Streams can be seen as an I/O streaming abstraction (maybe I'm using I/O in a more general sense than you). In any case, I don't see a reason for the abstraction to specifically not support something that's important for one of its implemented data sources (i.e. sockets). Again, the analogy to Unix read/write holds well - a file descriptor can correspond to pretty much anything - like a file on a ram drive, in which case it resembles a MemoryStream. The fact that it's a system call implemented in the kernel doesn't seem very important.

Knowing when you can read/write again is a concern that is totally separate from this question. Maybe you're using Socket.Select (already exposed), maybe you've wrapped epoll/kqueue, or maybe you're just trying opportunistically from time to time (in which case there's no underlying polling implementation at all).

That seems like a 1/2 baked abstraction to me.

I have no idea why that's half-baked - this is just the way things have been done in the Unix world for quite a while. Why not separate the polling mechanism (select vs. epoll vs. nothing) from the mechanism you later use to perform actual I/O? Can you point to any specific problem?

Can you think of any example in any other framework that has a concept of a non blocking write that isn't directly related to sockets?

I'm glad you asked... As I said above, the Unix read/write system calls are a good example, though they're not a framework (although I'm not sure why that should matter). So let's look at something that came out of the Unix world rather than out of the Windows world: Python. The standard Python streams abstraction is the io module - note that they use the terms I/O/streaming/file pretty interchangeably there. The basic write method returns an int specifically to support non-blocking, and when the socket isn't in non-blocking mode, always returns what the user requested to write.

Note that even the Python equivalent of BufferedStream, BufferedIOBase, contains specific handling for the case where its underlying raw stream is in non-blocking mode. In other words, support exists at multiple levels of the stream abstraction class hierarchy.

@damageboy
Copy link
Contributor

I'm sorry for barging in, but I don't see an unbreakable relationship between polling and non-blocking.

One could easily argue for opportunistic I/O in unix with no need to ever poll on the application level, but rather just use the returned amount of written octets...

For example, some systems use the following internal I/O pattern when interacting with the user:

  someObject.BeginSomeIO(); // Switches the socket to Non-Blocking
  someObject.Write(...);    // Piles up into a buffer, attempts to write it opportunistically
  someObject.Write(...);    // ditto...
  someObject.Write(...);
  someObject.FinishIO();    // Internally, switch the socket back to blocking, issue a sync write

This would be a classic case where just getting Write() to give the caller the amount of written bytes and the ability to know where the final sync Write() needs to start writing from.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@davidfowl, @stephentoub, now that new Span-based overloads are being added to Stream, would you consider changing the return value of sync Write from void to int, to make it compatible with the sync non-blocking I/O model? If I submit a PR on this would you consider accepting it?

It's really now or never, as releasing 2.1 without this will lock void as the return type forever. Once again, coming from the Unix world I can say it's a common pattern regardless of how you poll on the sockets (select/epoll/kqueue/whatever), and can be very effective.

@davidfowl
Copy link
Member

IMO this isn't something you would change on Stream. It could make sense to expose another API on socket directly though. Also are you going to use this with select/poll? There's no way to access the epoll or kqueue handle that the framework is using directly so I'm not sure what you would do when you get back EWOULDBLOCK. Are you suggesting that in order to use this new API the polling is up to the caller to figure out?

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@davidfowl unless I'm mistaken, all overloads of Socket.Send() already returns an int (e.g. this one) - the point is to expose this in the stream abstraction. It might also be a good idea to think about other I/O abstractions as well (Channels? Pipelines? I don't them well enough yet).

Here are some possible motivations for doing this:

  • It may be useful even in cases where no polling needs to happen - a single socket scenario. For example, an application may want to process data and push it to some remote host. It opportunistically tries to push data once it has some available (via a non-blocking sync write), and if it gets EWOULDBLOCK it goes back to processing more. This could definitely be implemented via async instead, but non-blocking sync has the advantage of being extremely simple, use exactly one thread and possibly be more efficient (no async overhead anywhere).
  • You guys may end up deciding to expose APIs like epoll/kqueue in .NET in the future, even if they're not exposed at the moment (select is exposed btw). They could be platform-specific APIs - after all you're releasing Windows registry APIs for .NET Core, so why not allow people to easily do epoll in Linux-only applications? Or you may end up doing some sort of higher-level crossplat abstraction over epoll/kqueue (like a .NET libuv). Either way, changing Write's return value to int would enable those scenarios.
  • Finally, for me it's more a matter of exposing a standard, basic I/O feature in the I/O abstraction for whatever purpose users end up using it.

What do you think?

@roji
Copy link
Member Author

roji commented Feb 25, 2018

And as a last comment - yes, the general idea here is to separate how you read/write from how you poll, after all these really are two separate concerns at the system call level. So I'm not concerned too much with polling here, only with allowing you to write in a non-blocking, synchronous manner.

(and I'll be happy to submit a PR for this if you guys want)

@davidfowl
Copy link
Member

I don't think you'd be able to implement this reasonably on anything except for NetworkStream (which would just call the underlying socket API) so I don't see how this can fit on Stream.

What would the default implementation do? How would each of the Streams in corefx implement these methods?

@roji
Copy link
Member Author

roji commented Feb 25, 2018

It's true that in most cases this method would always write the full amount requested, and would therefore simply return that, so I'd expect the default implementation in Stream to simply return count.

Although this is mostly a networking-only feature, it makes sense even for files. The man page for Linux's write syscall states that:

The number of bytes written may be less than count if, for example, there is insufficient space on the underlying physical medium, or the RLIMIT_FSIZE resource limit is encountered (see setrlimit(2)), or the call was interrupted by a signal handler after having written less than count bytes. (See also pipe(7).)

(I'm actually not sure how .NET's FileStream currently behaves on insufficient space).

Also, stream implementation which layer on top of NetworkStream could also implement proper sync non-blocking behavior (encryption, compression...). To be honest, this would require Stream to expose an IsBlocking to let wrapping streams know which mode they're on - but there's nothing urgent about doing that now (as opposed to Write's return type). So although it's somewhat network-specific, it definitely doesn't seem NetworkStream-specific.

Finally, if it seems odd that this is mostly network-related, most I/O or stream abstractions seem to work this way - Unix's write system call returns an int, although it can represent a file or basically anything else (exactly like .NET's stream). And the same is true of Python's io module. To be honest, this is just the way I'm used to seeing sync write functions, at least in the Unix/POSIX world.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

Here's one more non-NetworkStream scenario - in Unix you have named pipes, which look like files but are in fact "networking" to another process. So in .NET you'd perform I/O via FileStream and not NetworkStream, but it would still make sense to do non-blocking (it's networking in the sense that you're talking to another process, and the buffer between you can get full). So that's sync non-blocking I/O using FileStream.

(you'd still need to do fcntl to actually set the file descriptor to non-blocking somehow, but that's another question).

@davidfowl
Copy link
Member

I personally think it's a mismatch with our model and introducing more virtual methods with a different model on stream isn't desirable. .NET's model is async and there are lots of improvements like this one https://github.com/dotnet/corefx/issues/27445, that will make things even cheaper WRT async costs.

PS: What are you supposed to do in python when you get a blocking error back (BlockingIOError)?

@davidfowl
Copy link
Member

What about a TryWrite, TryRead API? That sorta fits the paradigm? I still have questions about what you do when it fails and would like to see how you actually write an end to end that lets you write code that waits until you're in a state where Try* will succeed.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

I personally think it's a mismatch with our model and introducing more virtual methods with a different model on stream isn't desirable. .NET's model is async and there are lots of improvements like this one dotnet/corefx#27445, that will make things even cheaper WRT async costs.

Well, we're not talking about introducing more virtual methods - all I'm asking for is to change the return type of one you've already added (but haven't released yet).

I think .NET's async focus is great, but shouldn't preclude us from supporting other I/O models - especially one that is so standard (at least in the Unix world). One simply doesn't exclude the other. And I wouldn't be pushing if it weren't such a small and easy change - it's just a return value change with almost no impact.

I also don't see how the .NET model is different in any way from the other models (POSIX syscall, python and many others), in a way that makes this a mismatch...? The stream API has existed since before async was introduced, and there's no reason not to continue support sync (especially as it's so cheap/easy).

PS: What are you supposed to do in python when you get a blocking error back (BlockingIOError)?

That really depends on your application. You may go into polling (with select/epoll/whatever), you may go back to computation, buffering more output data and try again in a bit of time, you may show a dialog to the user to let them decide what to do...

What about a TryWrite, TryRead API? That sorta fits the paradigm? I still have questions about what you do when it fails and would like to see how you actually write an end to end that lets you write code that waits until you're in a state where Try* will succeed.

Yeah, we briefly discussed that above. It doesn't work here because write isn't something that can just synchronously succeed or fail - it can also write partially (you wanted to write 10 bytes, but succeeded writing 5 bytes). So a simple bool doesn't work.

PS Don't forget the file scenario - I still don't know exactly what FileStream would do on Linux when trying to write and there's insufficient space, that may need to be addressed.

@davidfowl
Copy link
Member

davidfowl commented Feb 25, 2018

Well, we're not talking about introducing more virtual methods - all I'm asking for is to change the return type of one you've already added (but haven't released yet).

I think that's a really bad idea to introduce another model without thinking through the end to end. It's super late in 2.1 to change the Write on Stream to mean something entirely different and plumb that through the entire BCL.

One simply doesn't exclude the other. And I wouldn't be pushing if it weren't such a small and easy change - it's just a return value change with almost no impact.

It's not an easy change to make unless you're literally just talking about the base implementation and nothing else but that's not how these types of changes work.

I also don't see how the .NET model is different in any way from the other models (POSIX syscall, python and many others), in a way that makes this a mismatch...? The stream API has existed since before async was introduced, and there's no reason not to continue support sync (especially as it's so cheap/easy).

It's another model to teach people to use and its incomplete because there's no sane way to poll anything except for sockets so IMO this API is unusable until we have an end to end there. The other systems you mention expose APIs to poll efficiently as part of their end to end story, this is a 1/2 proposal IMO.

Yeah, we briefly discussed that above. It doesn't work here because write isn't something that can just synchronously succeed or fail - it can also write partially (you wanted to write 10 bytes, but succeeded writing 5 bytes). So a simple bool doesn't work.

bool TryWrite(Span<byte> buffer, int outWritten);

Wouldn't that work? What does the caller do with the unwritten buffer? I guess that's up to them? IMO these types of API proposals would do well to go with end to end sample showing how things are today and how they would look with these APIs. I've played with a bunch of the other APIs on various platforms (specifically java NIO) and they feel more complete than what you're proposing here.

PS Don't forget the file scenario - I still don't know exactly what FileStream would do on Linux when trying to write and there's insufficient space, that may need to be addressed.

We're using blocking File IO on linux (AFAIK), it'll just hang on a thread pool thread until the bytes are written.

@geoffkizer
Copy link
Contributor

I've read through the thread and I'm trying to distill the ask(s) here.

First, we have a nonblocking I/O model at the Socket level. I believe it does what you want, more or less; if there are improvements we could make here, we could certainly consider them.

Second, we can't change Stream.Write[Async] to return an int indicating how many bytes were written. Doing so would imply that the Write operation may succeed without writing all the bytes, requiring users to check the return value and handle it.

Third, the main ask here (if I'm understanding) seems to be to include a non-blocking I/O model in Stream itself -- not just in NetworkStream. That is, you want non-blocking to work for things like SslStream and DeflateStream etc. This seems like it would require exposing Poll (or equivalent) through the Stream abstraction. If this is what you want, an API proposal would be helpful.

That said, adding this would likely be a lot of work, so we'd need to be clear about what benefit you'd get from this. I get that it's like classic Unix poll-based I/O models; but, poll-based models don't have the equivalent of a Stream abstraction, so how would forcing these two things together make sense?

@roji
Copy link
Member Author

roji commented Feb 25, 2018

Well, we're not talking about introducing more virtual methods - all I'm asking for is to change the return type of one you've already added (but haven't released yet).

I think that's a really bad idea to introduce another model without thinking through the end to end. It's super late in 2.1 to change the Write on Stream to mean something entirely different and plumb that through the entire BCL.

It's true that it's late, although I did file this issue back in June, and you guys still seem to be doing things (e.g. https://github.com/dotnet/corefx/issues/27445).

I really don't see why this changes Write to mean something entirely different... Write literally means the same thing - it's just that in some real-life situations it can write less than requested, and this needs to be communicated to the user. Of course this needs to be thought through (that's what this conversation is for) - but I'm not seeing any catastrophic results in changing the return value from void to int on an unreleased method. Users calling this method are free to to ignore its return value if they want, just as it isn't returned today anyway.

Can you give an example of where this can conceivably create issues?

It's not an easy change to make unless you're literally just talking about the base implementation and nothing else but that's not how these types of changes work.

My concern here is first and foremost to change the return type, because that's going to be impossible later. It's even fine to change only the base implementation and not actually have NetworkStream return the real number of bytes written - that can be done later. Again, I have a hard time seeing how this could be so dangerous.

I also don't see how the .NET model is different in any way from the other models (POSIX syscall, python and many others), in a way that makes this a mismatch...? The stream API has existed since before async was introduced, and there's no reason not to continue support sync (especially as it's so cheap/easy).

It's another model to teach people to use and its incomplete because there's no sane way to poll anything except for sockets so IMO this API is unusable until we have an end to end there. The other systems you mention expose APIs to poll efficiently as part of their end to end story, this is a 1/2 proposal IMO.

I'm not getting this...

  1. You can poll in .NET with select, it's just not as efficient as epoll when a large number of sockets are involved. It's perfectly reason to select with a small number
  2. Even if it's a 1/2 proposal, you can always add epoll later - but you cannot change the return value later. I think exposing efficient polling APIs (epoll/kqueue) is great, but it will be too late to change write by the time you do.
  3. I'm sorry, but it's simply untrue to say that this is unusable with epoll/kqueue. You guys may be concentrating on kestrel and that's great, but it's not the entire universe. I've already provided some scenarios where this makes sense (single-socket operation where async isn't desired, simply checking that a file write was partial because of exhausted disk space).
  4. You don't have to teach people to use anything - sync non-blocking is another option in the toolbox, nobody has to use it if they don't want to.

It's really super simple. Every single write API I see, absolutely everywhere, returns an int of bytes written - except for .NET's stream. Everywhere, including Windows socket send and even WriteFile. Why so much pushback on such a universal API detail?

Please try to look outside the way things are done in Windows, and outside web server scenarios. It feels I'm really repeating myself, but sync non-blocking I/O is a very common model with its advantages and disadvantages, and we're about to block it just because of the return type of write (no pun intended!).

bool TryWrite(Span buffer, int outWritten);
Wouldn't that work? What does the caller do with the unwritten buffer? I guess that's up to them? IMO these types of API proposals would do well to go with end to end sample showing how things are today and how they would look with these APIs. I've played with a bunch of the other APIs on various platforms (specifically java NIO) and they feel more complete than what you're proposing here.

I'm guessing you meant to have an out variable called written right? Yes, that API would work, but it's a very confusing proposal - you have both the concept of success/failure (the returned bool) and a number of bytes. If you manage to write nothing, is that a failure while if you manage to write only 1 bytes of 10, that's a success? Or is it a success only when you manage to write the full 10 bytes, and anything less is a failure? What is the advantage in adding your API vs. the more standard way of just returning an int?

All this seems very very unnecessary when you could just return the value. And once again, write returning an int is a very standard feature which many developers already know when moving from other languages.

PS Don't forget the file scenario - I still don't know exactly what FileStream would do on Linux when trying to write and there's insufficient space, that may need to be addressed.

We're using blocking IO on linux, it'll just hang on a thread pool thread until the bytes are written.

We may be misunderstanding each other... If you try to write 10 bytes to a normal file with FileStream, but there's only 5 bytes left on the device (or on your quota), the Linux man page above says that the write system call will return 5. That is another (non-network) example of why write should return an int, rather than a void. I'm curious to know what happens in this scenario, and how you communicate to the user that his data was only partially written (you can't block a thread because it's not a temporary network TCP zero window, there's literally no more space left on the device).

@davidfowl
Copy link
Member

I'm sorry but I'm not convinced it fits. Maybe with a prototype and some sample code it'd be more convinced. Maybe grab some code from NpgSQL that you have today and rewrite it with this new API and show something you would do that you can't do today (or is ugly and expensive because of async).

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@geoffkizer, our messages crossed paths here...

First, we have a nonblocking I/O model at the Socket level. I believe it does what you want, more or less; if there are improvements we could make here, we could certainly consider them.

That's right, this is about exposing non-blocking in the streams abstraction. I brought two examples above where this also concerns FileStream (writing to an almost-full device, file access to named pipes), in an attempt to show how this is a general I/O abstraction issue rather than a networking-only issue.

Second, we can't change Stream.Write[Async] to return an int indicating how many bytes were written. Doing so would imply that the Write operation may succeed without writing all the bytes, requiring users to check the return value and handle it.

That's correct, but the only way in which the number of bytes written would be different from the requested value would be if non-blocking sockets have been turned on (currently unsupported by NetworkStream, checked at construction), and perhaps writing a file to an almost-fulldevice.

In other words, I can't see how there's a breaking change here: you have the option to turn on on non-blocking sockets, and then of course it's your responsibility to check the returned value. Otherwise, you in blocking mode, everything's working as usual and there's no need to look at the return value. Also, don't forget we're only discussing modifying a new method here - there's no code written against it yet.

Third, the main ask here (if I'm understanding) seems to be to include a non-blocking I/O model in Stream itself -- not just in NetworkStream. That is, you want non-blocking to work for things like SslStream and DeflateStream etc.

It's true I want to make it possible to do non-blocking in Stream rather than NetworkStream (see the two file-based examples above).

Regarding SslStream and DeflateStream, in theory it's indeed possible to make them work on non-blocking sockets. However, that's a whole other story, and I wouldn't expect you or anyone else to jump into it. The point is to make it possible by not hiding the number of bytes written, which is what the API currently does.

This seems like it would require exposing Poll (or equivalent) through the Stream abstraction. If this is what you want, an API proposal would be helpful.

Unless I'm misunderstanding, that's not true... I think there's a fundamental conflation here between writing and polling. It's fine to just ask to write up to a number of bytes, and if that fails, do something else rather than poll on the socket. Or if you do want to poll, select() is already there for you to use in .NET, and epoll/kqueue could be added in the future.

That said, adding this would likely be a lot of work, so we'd need to be clear about what benefit you'd get from this. I get that it's like classic Unix poll-based I/O models; but, poll-based models don't have the equivalent of a Stream abstraction, so how would forcing these two things together make sense?

I don't think that's true. As I wrote above, Python has a stream abstraction that supports polling non-blocking - there's no real reason not to. Also, when you're using POSIX syscalls such as write, you're effectively using a stream abstraction - Unix file descriptors can represent sockets, files or memory.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@davidfowl,

I'm sorry but I'm not convinced it fits. Maybe with a prototype and some sample code it'd be more convinced. Maybe grab some code from NpgSQL that you have today and rewrite it with this new API and show something you would do that you can't do today (or is ugly and expensive because of async).

OK, fair enough... I tried... :)

It would be interested to hear some more opinions (@stephentoub?), but if you're not convinced about this we can close.

@geoffkizer
Copy link
Contributor

In other words, I can't see how there's a breaking change here: you have the option to turn on on non-blocking sockets, and then of course it's your responsibility to check the returned value. Otherwise, you in blocking mode, everything's working as usual and there's no need to look at the return value.

This is only true if you assume that the consumer of the stream has control over whether the stream is blocking or nonblocking. In practice this isn't the case. Layered streams like SslStream and DeflateStream assume that Write will either write all bytes or fail.

Also, don't forget we're only discussing modifying a new method here - there's no code written against it yet.

Yes, but the new overloads don't modify the basic semantic of Write. You're talking about modifying the basic semantic of Write. This is no longer Write -- it's TryWrite, or something like that.

I think there's a fundamental conflation here between writing and polling. It's fine to just ask to write up to a number of bytes, and if that fails, do something else rather than poll on the socket.

Sure, you can go do something else. But eventually you're going to want to retry the write. Without Poll or something equivalent, you don't know when it's reasonable to retry the write, and that's not a very usable model.

Python has a stream abstraction that supports polling

Interesting, I didn't know that. I'll go spelunk it.

there's no real reason not to

The reason not to is, it's a lot of work to plumb the poll abstraction through every single Stream implementation, and it's not clear that you've really gained anything over async by doing so.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

In other words, I can't see how there's a breaking change here: you have the option to turn on on non-blocking sockets, and then of course it's your responsibility to check the returned value. Otherwise, you in blocking mode, everything's working as usual and there's no need to look at the return value.

This is only true if you assume that the consumer of the stream has control over whether the stream is blocking or nonblocking. In practice this isn't the case. Layered streams like SslStream and DeflateStream assume that Write will either write all bytes or fail.

I think that in general, yes, the stream consumer is supposed to control whether the stream is blocking or nonblocking. These are two fundamentally different I/O models and applications need to be architected differently to support them. I don't think anyone expects an I/O abstraction to work in the same way regardless of the mode of the underlying stream.

I'm only proposing the standard behavior in Unix (and possibly also Windows which I don't know): if you set your socket to non-blocking, write may return less than was requested (including zero).

Also, don't forget we're only discussing modifying a new method here - there's no code written against it yet.

Yes, but the new overloads don't modify the basic semantic of Write. You're talking about modifying the basic semantic of Write. This is no longer Write -- it's TryWrite, or something like that.

I guess you could see it that way, although I'd say the rest of the world tends to disagree... Again, in Unix, python and the rest the same write function is used for both "semantic behaviors", the blocking and non-blocking one.

I think there's a fundamental conflation here between writing and polling. It's fine to just ask to write up to a number of bytes, and if that fails, do something else rather than poll on the socket.

Sure, you can go do something else. But eventually you're going to want to retry the write. Without Poll or something equivalent, you don't know when it's reasonable to retry the write, and that's not a very usable model.

I don't agree. First, the whole point is that non-blocking I/O itself is a way to poll the socket... In other words, I could simply come back later (after some computation) and re-attempt to do the non-blocking write. Some applications may simply delegate this decision to the user via a UI - the user arbitrarily decides when to attempt to send data, and the attempt simply sends as much as possible each time.

Finally, it's true that in most cases you do want to poll. For those cases select() does exist in the API, and as mentioned above I don't see a reason why epoll and/or kqueue wouldn't be exposed one day in the future - the idea of this issue to have other elements in place for when that happens.

Python has a stream abstraction that supports polling

Interesting, I didn't know that. I'll go spelunk it.

there's no real reason not to

The reason not to is, it's a lot of work to plumb the poll abstraction through every single Stream implementation,

I may have missed something, but all I'm proposing is to have the default new Write(ReadOnlySpan<byte>) on Stream return the requested number of bytes (i.e. written = requested). Implementing stream implementations, assuming they override Write(ReadOnlySpan<byte>), would simply have to do the same if they doesn't support writing less than requested. That seems like a really trivial and low-risk change (which I'd be happy to do), but again, I may be missing something.

and it's not clear that you've really gained anything over async by doing so.

This is where I think the discussion is blocking - it seems that with all the attention async is (deservedly!) receiving this other alternative model is, well, being simply rejected, even if supporting it doesn't imply much effort... I for one believe that despite its advantages, async is far from being free - the need to think about different threads, synchronization contexts, thread pool and possible starvation and all the rest are very non-trivial.

But as I wrote above I can let this go, there are definitely more important things in the world...

@geoffkizer
Copy link
Contributor

the rest of the world tends to disagree

I think you're focusing way too much on this.

It doesn't matter how the rest of the world works. Not because the existing Stream is "right", or Unix is "wrong", or anything like that. The point is simply that Stream works the way it works. You're asking for it to work differently, and that's a big deal.

the idea of this issue to have other elements in place for when that happens

This is the wrong way to go about it. If we want to design Poll for Stream, let's do that. Doing part of the design, and then hoping it will work if/when we do the rest of the design, is not the right approach.

all I'm proposing is to have the default new Write(ReadOnlySpan) on Stream return the requested number of bytes

Again, this is not a small change, for the reasons I've listed above. We would not change the new API. We could consider a new API like TryWrite, but again, this seems like a half-measure unless you have a full proposal for handling Poll or similar.

this other alternative model is, well, being simply rejected, even if supporting it doesn't imply much effort

I'm not rejecting alternative models. I'm trying to explore how they would actually work.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

the rest of the world tends to disagree

I think you're focusing way too much on this.

It doesn't matter how the rest of the world works. Not because the existing Stream is "right", or Unix is "wrong", or anything like that. The point is simply that Stream works the way it works. You're asking for it to work differently, and that's a big deal.

I understand what you're saying, but I see things differently. .NET Streams (at least the part we're discussing) isn't some sort of unique product you guys have created - it follows the I/O patterns that everybody has been applying for decades. Your methods are called read and write, seek and flush; your read and write methods accept a byte array, offset and length like all the other dozens of I/O abstractions out there. It's a de facto standard.

And here's the thing - your methods are called read and write, which are standard names for standard functionality and semantics, and part that standard is, well, that write returns an how much its written. So I think there definitely is a strong justification for looking at what the world does, because everything else about that API is standard.

Now, of course when I originally opened this issue it was largely theoretical, because it's impossible to change the return value of existing method. But with the new Span-based method there's an actual opportunity to fix it and align with the way other I/O APIs work, rather than to come back later and bolt yet another overload on it.

the idea of this issue to have other elements in place for when that happens

This is the wrong way to go about it. If we want to design Poll for Stream, let's do that. Doing part of the design, and then hoping it will work if/when we do the rest of the design, is not the right approach.

OK, except that I'm not talking about polling (select/epoll/kqueue), I'm talking about non-blocking writes. Unlike the former, the latter have a standard way of being exposed in I/O APIs. If you're claiming the latter have little value without the former, then I don't necessarily agree, but the important thing is that the two are distinct things which can be designed separately.

I do promise that I wouldn't be pushing to do this now if we didn't have the opportunity right in front of us.

all I'm proposing is to have the default new Write(ReadOnlySpan) on Stream return the requested number of bytes

Again, this is not a small change, for the reasons I've listed above. We would not change the new API. We could consider a new API like TryWrite, but again, this seems like a half-measure unless you have a full proposal for handling Poll or similar.

I simply disagree. Just like you can do ordinary synchronous blocking I/O without select/epoll/kqueue, you can do synchronous non-blocking I/O without them. It's really unclear to me why you guys insist on connecting the two - it may not happen in the web server space you're currently concentrating on, but there are other scenarios out there.

If you end up with a new API like TryWrite, fine - you will have added non-blocking writes to .NET. But you will have missed an opportunity to do it in the simple, intuitive way every other API out there has done it for the past 40 years, and I don't think you won't have much to show for it (but I may be wrong).

this other alternative model is, well, being simply rejected, even if supporting it doesn't imply much effort

I'm not rejecting alternative models. I'm trying to explore how they would actually work.

I understand and I'm sorry if that came across as aggressive, but you did suggest that nothing would be gained over async here. So there's the question of API shape, and there's the general question of people not thinking its worth thinking about non-blocking writes at all.

@geoffkizer
Copy link
Contributor

part that standard is, well, that write returns an how much its written.

Ok, let's take that as a given.

It still doesn't matter.

Stream.Write in .NET is defined to behave the way it behaves; that is, write all or fail.

We cannot change the definition at this point.

We can add TryWrite or whatever, but we cannot change the definition of Write.

I feel like we've gone around on this, but we're not converging.

Again, I'm happy to discuss how to support polling-based models better; but I'm not sure we're really getting anywhere here.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@geoffkizer it's OK, I also agree that we've gone around this and aren't likely to converge.

I guess the main question here is what your constraints are when adding a new overload to a method - to what extent you're allowed to depart from the way the existing behaves. I do accept your point that having a new overload of Write() which supports non-blocking alongside old overloads which don't is a bit odd. On the other hand the new async overloads return ValueTask rather than Task, which might not be a big/semantic change but is still inconsistent, in a way (imagine a user switching across overloads and seeing an unexplicably different allocation profile). I guess I'm just willing to go further than you are, but I do understand what you're saying.

Since you seem to open/interested in this, I can leave this issue for future discussions of what a new API for non-blocking I/O would look like.

@davidfowl
Copy link
Member

I think there are various ways that are more or less idiomatic to ecosystem at hand. For .NET IMHO I think a TryWrite would "fit" more readily than suddenly changing the meaning of Write to non blocking because we have a new overload.

If we do go this route, I would still love to see what code samples would look like that used this pattern (do you flip between TryWrite and WriteAsync/Write?). FWIW, libuv, even though it's a callback based model through and through (they implement it on top of non blocking IO because it's pretty easy to do so) expose a try_write (http://docs.libuv.org/en/v1.x/stream.html).

int uv_try_write(uv_stream_t* handle, const uv_buf_t bufs[], unsigned int nbufs)
Same as uv_write(), but won’t queue a write request if it can’t be completed immediately.

Will return either:

> 0: number of bytes written (can be less than the supplied buffer size).
< 0: negative error code (UV_EAGAIN is returned if no data can be sent immediately).

PS: This thread is pretty good https://stackoverflow.com/questions/41904221/can-write2-return-0-bytes-written-and-what-to-do-if-it-does.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

I think there are various ways that are more or less idiomatic to ecosystem at hand. For .NET IMHO I think a TryWrite would "fit" more readily than suddenly changing the meaning of Write to non blocking because we have a new overload.

Fair enough.

As I wrote above, I really that .NET TryXXX is really not well-suited here, because it's about operations that can succeed or fail, and not execute partially. It's very unclear how the success/failure bool would interact with the actual number of bytes written. But of course another API could be found...

If we do go this route, I would still love to see what code samples would look like that used this pattern (do you flip between TryWrite and WriteAsync/Write?). FWIW, libuv, even though it's a callback based model through and through (they implement it on top of non blocking IO because it's pretty easy to do so) expose a try_write (http://docs.libuv.org/en/v1.x/stream.html).

That's interesting, thanks!

PS: This thread is pretty good https://stackoverflow.com/questions/41904221/can-write2-return-0-bytes-written-and-what-to-do-if-it-does.

Hmm, that seems to be focused more about cases where 0 if returned, and a bit less on partial writes, but thanks - it's interesting too.

BTW I'll try to do a test to see how .NET Core behaves on Linux when a write cannot complete fully because of quota or disk space

@stephentoub
Copy link
Member

It would be interested to hear some more opinions (@stephentoub?)

I agree with @davidfowl and @geoffkizer. We should not change this. Regardless of overload, Stream.Write{Async} writes all of the data given to it, plain and simple. That's how it's always been. Making it so that one overload differs in that behavior, and differs in a way that I guarantee you will lead to a trove of bugs, is simply not tenable.

I'd be fine entertaining the notion of a TryWrite or a WritePartial or some such thing for the future, though I'd want to see the idea fully fleshed out.

@roji
Copy link
Member Author

roji commented Feb 25, 2018

@stephentoub thanks for your commenting on this... will leave this issue open as there seems to be at least some interest in partial writing.

@StephenCleary
Copy link
Contributor

I've used select/poll-style, both on UNIX and Windows. POSIX AIO was unfortunately just too awkward of an API coming too late to the ecosystem.

But as soon as I had the chance to use OVERLAPPED, I switched right to that. The programming concepts are much easier to deal with. Then moved to .NET, with an IOCP built-in and Task replacing OVERLAPPED, and it's even easier. And finally async/await are wonderful sugar.

I have a hard time thinking of a single example where select/poll would be more maintainable. And with IOCP, I have a hard time thinking of a real-world example where select/poll would be more efficient, either.

@roji
Copy link
Member Author

roji commented Jun 3, 2019

Hey @StephenCleary!

It's true that I come from a Unix system programming background - that may explain why non-blocking (synchronous) I/O appeals to me. As opposed to asynchronous I/O, the model is extremely simple and a lot of complexity is obviated - no need to think which thread is going to run your callback, no complicated stacks to wade through and reconstruct program flow from, no issues with transparent thread-local state... As much as I love async/await (and I do love it), it is not free in terms of conceptual complexity and gotchas.

The simplicity of the non-blocking model can sometimes translate into performance benefits as well - I did some playing around a few years back. There can (sometimes!) be less need for context switching, more affinity between the thread, its CPU and the data structures for the set of connections being handled, and synchronization needs are sometimes obviated as well as the threading model is simpler and more under the programmer's direct control.

Anyway, as there has been very little interest (or even negative interest) I'll go ahead and close this now. In any case .NET does have Socket.Blocking which provides the basic functionality.

@roji roji closed this as completed Jun 3, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net design-discussion Ongoing discussion about design without consensus enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

8 participants