4

I am using NetworkStream with TcpClient.

  1. First I setup my tcp client:

        tcp = new TcpClient(AddressFamily.InterNetwork)
        { NoDelay = true, ReceiveTimeout = 5000};
    
  2. My main data-receiving loop:

        while (true)
        {
            //read available data from the device
            int numBytesRead = await ReadAsync();
            Console.WriteLine($"{numBytesRead} bytes read"); //BP2
        }
    
  3. And the actual TCP data reading:

    public Task<int> ReadAsync()
    {
        var stream = tcp.GetStream();
        return stream.ReadAsync(InBuffer, 0, InBuffer.Length); //BP1
    }
    

I have this connected to a testbed which lets me send manual packets. Through setting breakpoints and debugging I have checked that stream.ReadTimeout takes the value 5000 from tcp.

If I send data frequently it all works as expected. But if I don't send any data, nothing appears to happen after 5s, no timeout. I see breakpoint BP1 being hit in the debugger but until I send data from my testbed, BP2 is not hit. I can leave it a minute or more and it just seems to sit waiting, but receives data sent after a minute, which appears to be incorrect behavior. After 5 seconds something should happen, surely (an exception as I understand it)?

It's late so I am expecting something really basic but can anyone see what my mistake is and a resolution?


Addendum

OK so when I RTFM for the actual .Net version I'm using (how may times have I been caught out by MS defaulting to .Net Core 3, I did say it was late) I see in the remarks sectio for ReadTimeout:

This property affects only synchronous reads performed by calling the Read method. This property does not affect asynchronous reads performed by calling the BeginRead method.

I'm unclear now if I can use modern awaitable calls at all to read socket data safely and with a timeout specifically. It's working except for the timeout but I'm not sure how given ReadAsync has no override in NetworkStream. Must I do some ugly hack or is there a simple solution?

In my case 5000 is the longest I can expect not to receive data before concluding there is a problem - the protocol has no ping mechanism so if nothing appears I assume the connection is dead. Hence thinking an Async read with a 5000ms timeout would be nice and neat.

Mr. Boy
  • 60,845
  • 93
  • 320
  • 589
  • Probably related to https://stackoverflow.com/q/12421989/11683 – GSerg Jun 02 '20 at 21:32
  • @GSerg it looks like it could be but it's suggested to use timeouts in an answer there which is what I'm doing. It does mention `ReadAsync` is a non-overriden base method but I am lost at this point! – Mr. Boy Jun 02 '20 at 21:36
  • True is disable. – jdweng Jun 02 '20 at 21:38
  • That's an external timeout they are using, not intrinsic to the stream, which closes the stream from under `ReadAsync`. – GSerg Jun 02 '20 at 21:38
  • 2
    @jdweng I have no idea what you're saying here! – Mr. Boy Jun 02 '20 at 21:40
  • The Nodelay documentation says " true if the delay is disabled". You want the delay so the value should be false. – jdweng Jun 02 '20 at 21:47
  • 1
    @jdweng: this question has nothing to do with the `NoDelay` property. `NoDelay` affects whether the Nagel algorithm is enabled or not, and has zero relationship to timeouts on I/O operations. – Peter Duniho Jun 02 '20 at 22:12

1 Answers1

7

Timeout values for network objects apply only to synchronous operations. For example, from the documentation:

This option applies to synchronous Receive calls only.

For Socket.ReceiveTimeout, TcpClient.ReceiveTimeout, and NetworkStream.ReadTimeout, the implementations all ultimately result in a call to SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReceiveTimeout, ...) which in turn is effectively calling the native setsockopt() function. From that documentation:

SO_RCVTIMEO DWORD Sets the timeout, in milliseconds, for blocking receive calls.

(emphasis mine)

It's this limitation in the underlying native API that is the reason for the same limitation in the managed API. Timeout values will not apply to asynchronous I/O on the network objects.

You will need to implement the timeout yourself, by closing the socket if and when the timeout should occur. For example:

async Task<int> ReadAsync(TcpClient client, byte[] buffer, int index, int length, TimeSpan timeout)
{
    Task<int> result = client.GetStream().ReadAsync(buffer, index, length);

    await Task.WhenAny(result, Task.Delay(timeout));

    if (!result.IsCompleted)
    {
        client.Close();
    }

    return await result;
}

Other variations on this theme can be found in other related questions:
NetworkStream.ReadAsync with a cancellation token never cancels
Cancel C# 4.5 TcpClient ReadAsync by timeout

Closing the socket is really all that you can do. Even for synchronous operations, if a timeout occurs the socket would no longer be usable. There is no reliable way to interrupt a read operation and expect the socket to remain consistent.

Of course, you do have the option of prompting the user before closing the socket. However, if you were to do that, you would implement the timeout at a higher level in your application architecture, such that the I/O operations themselves have no awareness of timeouts at all.

Peter Duniho
  • 68,759
  • 7
  • 102
  • 136
  • So are we saying socket-receive is fundamentally a blocking operation, at the low level there simply _isn't_ an asyncronous alternative and all .Net can do is wrap it in a thread? I'm curious how `Stream.ReadAsync` knows when the blocking call has finsihed. Anyway if this is the only real option it's a _bit_ smelly but not too terrible. Thanks for the nice explaantion – Mr. Boy Jun 03 '20 at 08:44
  • The one "problem" with this is you get an `ObjectDisposedException` which I would guess might be easy to miss as you are typically handling `IOException` and `SocketException`. You could return `0` and the caller could decide to reconnect, but then you have a dangling read operation... not sure there _is_ aperfect solution. – Mr. Boy Jun 03 '20 at 11:24
  • 2
    @Mr.Boy: _"are we saying socket-receive is fundamentally a blocking operation, at the low level there simply isn't an asyncronous alternative"_ -- no, not at all. At the OS level, async operations are very much possible (see "overlapped I/O" and "I/O completion port"). It's just that timeouts are not supported when using async operations. ... – Peter Duniho Jun 03 '20 at 15:09
  • ... _"The one "problem" with this is you get an ObjectDisposedException"_ -- that shouldn't be a problem at all. Just include that type of exception in the ones you're catching. You shouldn't be handling exceptions in the read operation anyway; it needs to be at a higher level, where your code has the ability to know to give up on that connection and do something appropriate (whether to retry the connection, report an error to the user, etc.). – Peter Duniho Jun 03 '20 at 15:09
  • I'd tend to think catching exceptions as early as possible is preferable. I might have both a TCP and HTTP version of some connection class and their read-failure exceptions will be different - I'd like my calling code abstracted from the precise details rather than having to catch all sorts of exceptions. If that makes sense. I suppose I can catch in `ReadAsync` and re-throw a custom exception, or implement a `TryRead` sort of pattern – Mr. Boy Jun 03 '20 at 15:25
  • 1
    @Mr.Boy: _"I'd tend to think catching exceptions as early as possible is preferable"_ -- that's really just not true. You want to catch an exception _exactly where you can appropriately handle it_. Exception handling can be a pain in the neck, but one of the big advantages is that exceptions cross method-call boundaries trivially, allowing error handling to be put only where it's actually needed and appropriate. Following an "as early as possible" policy negates this benefit, one of the few benefits one even can say one gets from structured exception handling. – Peter Duniho Jun 03 '20 at 16:02
  • 1
    _"I'd like my calling code abstracted from the precise details rather than having to catch all sorts of exceptions"_ -- the way to do that is catch and rethrow with your more-specific, application-defined exception. That in no way suggests that adding a new exception such as `ObjectDisposedException` to the low-level handling is a problem. Indeed, given your goal of _"calling code abstracted from the precise details rather than having to catch all sorts of exceptions"_ you should be doing that anyway. – Peter Duniho Jun 03 '20 at 16:04
  • Sure - no arguments there. Thanks – Mr. Boy Jun 03 '20 at 16:14