Socket.SendFile should fail if when Socket.Blocking = false

[geoffkizer]

If you put a socket into nonblocking mode (i.e. set Socket.Blocking=false) and then call synchronous SendFile, you get unexpected behavior.

On Windows, the call will block but will ultimately succeed.
(EDIT: See my comment below for clarification.) On Linux, you will likely end up with an EWOULDBLOCK result, but this isn't not reliable; sometimes the call will succeed, sometimes it will send partial data, etc.

SendFile is not intended to be used in nonblocking mode. If a user attempts this we should throw InvalidOperationException.

More details here: #47230 (review)

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

If you put a socket into nonblocking mode (i.e. set Socket.Blocking=false) and then call synchronous SendFile, you get unexpected behavior.

On Windows, the call will block but will ultimately succeed.
On Linux, you will likely end up with an EWOULDBLOCK result, but this isn't not reliable; sometimes the call will succeed, sometimes it will send partial data, etc.

SendFile is not intended to be used in nonblocking mode. If a user attempts this we should throw InvalidOperationException.

More details here: #47230 (review)

Author:	geoffkizer
Assignees:	-
Labels:	area-System.Net.Sockets
Milestone:	-

[karelz]

Triage: We should throw as suggested above. Should be fairly straightforward to do.

[jaymin93]

Just to understand it better this bug fix would be checking for blocking =false in every synchronous method and throw in case of it is false need to throw invalid operation exception , is my understanding towards this bug is correct?

[tmds]

On Linux, you will likely end up with an EWOULDBLOCK result, but this isn't not reliable; sometimes the call will succeed, sometimes it will send partial data, etc.

On Linux this should work.

sendfile works the same as sendmsg: you need to call it again until everything was sent, and in case of EAGAIN/EWOULDBLOCK wait for poll.

TryCompleteSendFile is missing some logic that calls it again on partial sends, like what we have in TryCompleteSendTo.

[geoffkizer]

Yeah, my comment above is wrong/confusing.

I should have said, on Linux, the kernel will buffer as much of the file as it can/wants to, and then tell you how much it sent, just like any other send operation. This will likely result in EWOULDBLOCK if you need to call sendfile multiple times, but that's not the real problem.

The problem is, Socket.SendFile doesn't return a value telling you how much it actually sent, like Socket.Send does. So there's no way for the user to know how much of the file was actually sent, which makes the API pretty useless.

We could in theory add a new method that returns the bytesSent. But there's no obvious way to make this work on Windows, since Windows doesn't seem to actually respect the non-blocking state of the socket for TransmitFile.

I think the conclusion still stands: we should just fail Socket.SendFile when Socket.Blocking = false.

[geoffkizer]

TryCompleteSendFile is missing some logic that calls it again on partial sends, like what we have in TryCompleteSendTo.

In what case in particular? It looks like we are doing the retry logic in SocketAsyncContext.

[geoffkizer]

Just to understand it better this bug fix would be checking for blocking =false in every synchronous method and throw in case of it is false need to throw invalid operation exception , is my understanding towards this bug is correct?

No, this is specifically about SendFile.

[jaymin93]

Just to understand it better this bug fix would be checking for blocking =false in every synchronous method and throw in case of it is false need to throw invalid operation exception , is my understanding towards this bug is correct?

No, this is specifically about SendFile.

ok so i need to apply blocking =false check for specific method of send file and this will fix this bug , asking because i want to make pr , so before that i need to be clear about what actually case of bug fix, Thanks

[geoffkizer]

SendFile in when Socket.Blocking = false.

[tmds]

In what case in particular? It looks like we are doing the retry logic in SocketAsyncContext.

We're not looping on partial sends like we do for TryCompleteSendTo until there is nothing more to be sent:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

Lines 883 to 885 in 4df29c9

	bool isComplete = sent == 0 ||
	(buffers == null && count == 0) ||
	(buffers != null && bufferIndex == buffers.Count);

[geoffkizer]

We are looping in TryCompleteSendFile:

runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs

Line 935 in 4df29c9

if (sent == 0 || count == 0)

[tmds]

Ah yes.

Is there a test that occasionally fails on Linux that I can run?

[geoffkizer]

I don't think there's an existing test, but there's discussion of what tests we should have here: #47230 (review)

@antonfirsov Did we add a (disabled) test for this?

[vijaya-lakshmi-venkatraman]

Hi,
Please check if my below summary of the required changes is right:

• File : runtime/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs
• Function : TryCompleteSendFile
• Changes:

try 
{
     if(Socket.Binding == false)
     {
            sent = SendFile(socket, handle, ref offset, ref count, out errno);
            bytesSent += sent;
      }
}

1. Is this right?
2. Is it safe to assume that Socket.Binding is always available? I see that Socket.Binding is mentioned in one of the places

case FIONBIO:
                    // The Windows implementation explicitly throws this exception, so that all
                    // changes to blocking/non-blocking are done via Socket.Blocking.
                    throw new InvalidOperationException(SR.net_sockets_useblocking);