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

Cancellation on NetworkStream.ReadAsync closes local connection but remote server end-point still seems to be connected #69397

Closed
zahirtezcan-bugs opened this issue May 16, 2022 · 15 comments
Labels
area-System.Net.Sockets needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity
Milestone

Comments

@zahirtezcan-bugs
Copy link

Description

Using .NET 6 on Windows 10

For a multithreaded TcpClient one loop for receiving and other loop for sending messages, if we use NetworkStream.ReadAsync with a CancellationToken, and if the token gets canceled
1- ReadAsync throws a cancellation exception, which is expected (documentation of ReadAsync does not declare it though)
2- TcpClient.Connected becomes false, which is not-expected (or at least I did not)
3- TcpClient.Connected on server side (the client returned from TcpListener.AcceptClientAsync) stays true, which is not-expected.

Behavior remains same for

Reproduction Steps

For client/server scenario with TcpListener and TcpClient classes;

  • Two different processes one named Server and other named as Client, with prior configuration lets say listener is at 127.0.0.1:13337 and buffer-size is 24, also maintain a CancellationToken in each process to trigger the error. (I am using Microsoft.Extensions.Hosting and triggering cancellation via CTRL+C)
    • Server starts a TcpListener via Start method and waits for a client with AcceptTcpClientAsync
      • when an instance of TcpClient is received run an asynchronous loop via Task.Run
      • in the loop read buffer-size bytes via NetworkStream.ReadAsync and echo bytes back via NetworkStream.WriteAsync
        • Use ReadAsync in another loop to guarantee reading buffer-size bytes before writing back.
    • Client instantiates a TcpClient with empty constructor and enters a loop by calling ConnectAsync
      • after connection is established spawns two loops one for sending and one for receiving in different threads via Task.Run(is this OK?)
      • each loop uses NetworkStream.ReadAsync and NetworkStream.WriteAsync accordingly but WriteAsync works only a limited number of times to make ReadAsync eventually wait
        • remember the server is echoing so if we don't send then we won't receive
      • we print related messages to see that communication occurs flawlessly and eventually halts
      • when communication stops (so the client is waiting on ReadAsync) we trigger cancellation
      • ReadAsync exits via an OperationCanceledException but TcpClient.Connected returns to false and further communication is not possible.
  • Also, on server-side ReadAsync calls returns immediately with zero 0 read-count but TcpClient.Connected remains true on the server-side. Even if we call TcpClient.Close on the client-side, server-side will still report Connected as true
    • Before triggering the error server-side waits on ReadAsync as expected.

Expected behavior

  • If possible NetworkStream.ReadAsync should not affect connection status for a cancellation via CancellationToken
  • If NetworkStream.ReadAsync needs to close connection then it should do it properly so that listener side can close the client properly.

Actual behavior

  • When CancellationToken gets canceled while NetworkStream.ReadAsync is waiting on a message from network, the connection gets closed on the client-side
  • After client-side closes as described then server-side TcpClient.Connected still reports connected but its NetworkStream.ReadAsync always returns with zero 0 count immediately.

Regression?

No response

Known Workarounds

No response

Configuration

  • .NET 6
  • Windows 10
  • x64
  • Same error in Ubuntu 20.04 WSL2

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 16, 2022
@ghost
Copy link

ghost commented May 16, 2022

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

Issue Details

Description

Using .NET 6 on Windows 10

For a multithreaded TcpClient one loop for receiving and other loop for sending messages, if we use NetworkStream.ReadAsync with a CancellationToken, and if the token gets canceled
1- ReadAsync throws a cancellation exception, which is expected (documentation of ReadAsync does not declare it though)
2- TcpClient.Connected becomes false, which is not-expected (or at least I did not)
3- TcpClient.Connected on server side (the client returned from TcpListener.AcceptClientAsync) stays true, which is not-expected.

Behavior remains same for

Reproduction Steps

For client/server scenario with TcpListener and TcpClient classes;

  • Two different processes one named Server and other named as Client, with prior configuration lets say listener is at 127.0.0.1:13337 and buffer-size is 24, also maintain a CancellationToken in each process to trigger the error. (I am using Microsoft.Extensions.Hosting and triggering cancellation via CTRL+C)
    • Server starts a TcpListener via Start method and waits for a client with AcceptTcpClientAsync
      • when an instance of TcpClient is received run an asynchronous loop via Task.Run
      • in the loop read buffer-size bytes via NetworkStream.ReadAsync and echo bytes back via NetworkStream.WriteAsync
        • Use ReadAsync in another loop to guarantee reading buffer-size bytes before writing back.
    • Client instantiates a TcpClient with empty constructor and enters a loop by calling ConnectAsync
      • after connection is established spawns two loops one for sending and one for receiving in different threads via Task.Run(is this OK?)
      • each loop uses NetworkStream.ReadAsync and NetworkStream.WriteAsync accordingly but WriteAsync works only a limited number of times to make ReadAsync eventually wait
        • remember the server is echoing so if we don't send then we won't receive
      • we print related messages to see that communication occurs flawlessly and eventually halts
      • when communication stops (so the client is waiting on ReadAsync) we trigger cancellation
      • ReadAsync exits via an OperationCanceledException but TcpClient.Connected returns to false and further communication is not possible.
  • Also, on server-side ReadAsync calls returns immediately with zero 0 read-count but TcpClient.Connected remains true on the server-side. Even if we call TcpClient.Close on the client-side, server-side will still report Connected as true
    • Before triggering the error server-side waits on ReadAsync as expected.

Expected behavior

  • If possible NetworkStream.ReadAsync should not affect connection status for a cancellation via CancellationToken
  • If NetworkStream.ReadAsync needs to close connection then it should do it properly so that listener side can close the client properly.

Actual behavior

  • When CancellationToken gets canceled while NetworkStream.ReadAsync is waiting on a message from network, the connection gets closed on the client-side
  • After client-side closes as described then server-side TcpClient.Connected still reports connected but its NetworkStream.ReadAsync always returns with zero 0 count immediately.

Regression?

No response

Known Workarounds

No response

Configuration

  • .NET 6
  • Windows 10
  • x64
  • Same error in Ubuntu 20.04 WSL2

Other information

No response

Author: zahirtezcan-bugs
Assignees: -
Labels:

area-System.Net

Milestone: -

@ghost
Copy link

ghost commented May 16, 2022

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

Issue Details

Description

Using .NET 6 on Windows 10

For a multithreaded TcpClient one loop for receiving and other loop for sending messages, if we use NetworkStream.ReadAsync with a CancellationToken, and if the token gets canceled
1- ReadAsync throws a cancellation exception, which is expected (documentation of ReadAsync does not declare it though)
2- TcpClient.Connected becomes false, which is not-expected (or at least I did not)
3- TcpClient.Connected on server side (the client returned from TcpListener.AcceptClientAsync) stays true, which is not-expected.

Behavior remains same for

Reproduction Steps

For client/server scenario with TcpListener and TcpClient classes;

  • Two different processes one named Server and other named as Client, with prior configuration lets say listener is at 127.0.0.1:13337 and buffer-size is 24, also maintain a CancellationToken in each process to trigger the error. (I am using Microsoft.Extensions.Hosting and triggering cancellation via CTRL+C)
    • Server starts a TcpListener via Start method and waits for a client with AcceptTcpClientAsync
      • when an instance of TcpClient is received run an asynchronous loop via Task.Run
      • in the loop read buffer-size bytes via NetworkStream.ReadAsync and echo bytes back via NetworkStream.WriteAsync
        • Use ReadAsync in another loop to guarantee reading buffer-size bytes before writing back.
    • Client instantiates a TcpClient with empty constructor and enters a loop by calling ConnectAsync
      • after connection is established spawns two loops one for sending and one for receiving in different threads via Task.Run(is this OK?)
      • each loop uses NetworkStream.ReadAsync and NetworkStream.WriteAsync accordingly but WriteAsync works only a limited number of times to make ReadAsync eventually wait
        • remember the server is echoing so if we don't send then we won't receive
      • we print related messages to see that communication occurs flawlessly and eventually halts
      • when communication stops (so the client is waiting on ReadAsync) we trigger cancellation
      • ReadAsync exits via an OperationCanceledException but TcpClient.Connected returns to false and further communication is not possible.
  • Also, on server-side ReadAsync calls returns immediately with zero 0 read-count but TcpClient.Connected remains true on the server-side. Even if we call TcpClient.Close on the client-side, server-side will still report Connected as true
    • Before triggering the error server-side waits on ReadAsync as expected.

Expected behavior

  • If possible NetworkStream.ReadAsync should not affect connection status for a cancellation via CancellationToken
  • If NetworkStream.ReadAsync needs to close connection then it should do it properly so that listener side can close the client properly.

Actual behavior

  • When CancellationToken gets canceled while NetworkStream.ReadAsync is waiting on a message from network, the connection gets closed on the client-side
  • After client-side closes as described then server-side TcpClient.Connected still reports connected but its NetworkStream.ReadAsync always returns with zero 0 count immediately.

Regression?

No response

Known Workarounds

No response

Configuration

  • .NET 6
  • Windows 10
  • x64
  • Same error in Ubuntu 20.04 WSL2

Other information

No response

Author: zahirtezcan-bugs
Assignees: -
Labels:

area-System.Net.Sockets, untriaged

Milestone: -

@wfurt
Copy link
Member

wfurt commented May 16, 2022

Do you have simple repo @zahirtezcan-bugs ? I suspect we hit UpdateStatusAfterSocketError(SocketException socketException, bool disconnectOnFailure = true) and that sets the socket to disconnected state. You can probably Dispose the client to make the server disconnect as well. (you can also do packet capture to see what is going on on TCP level) I agree it would be nice if the state of Connected reflects the actual OS state.

As far as leaving the original state: It is tricky to ensure consistency as the cancelation can be triggered at different places. e.g. some bytes may be read from OS but not delivered to application. Preserving such data and order with multiple reads may be non trivial.

@zahirtezcan-bugs
Copy link
Author

I was trying this in my company network, sorry that I could not export actual code but I have added a repro and kept things simpler, for example client operates in single loop writes some data and reads in a loop indefinitely.

https://github.com/zahirtezcan-bugs/AsyncTcp

Note: In all of my tries I always closed the client via Close or Dispose (both have same effect AFAIK).

@antonfirsov
Copy link
Member

Triage: we should look into this, but not high-prio.

@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label May 17, 2022
@antonfirsov antonfirsov added this to the Future milestone May 17, 2022
@ExtremeGTX
Copy link

ExtremeGTX commented May 31, 2023

Thanks for reporting this problem, wasted an hour trying to figure out why the tcpClient.Connected changed to false when the ReadAsync operation got cancelled. is there a workaround for this issue ?

@zahirtezcan-bugs
Copy link
Author

zahirtezcan-bugs commented Mar 1, 2024

NetworkStream does not support non-blocking I/O (discussion here: #22506) and this issue prevents us using timeout-cancellation. Is there a way to use asynchronous API and perform an non-blocking-wait on a read operation with timeout?

@wfurt
Copy link
Member

wfurt commented Mar 1, 2024

What is your goal @zahirtezcan-bugs? I think you can make non-blocking work with NetworkStream. But also looking at the provided example I see not much benefit of using the stream instead of Socket directly.

The Connected behavior look suspicious and I'll look into it. I would think one can build the logic completely without relying on the property. With the thread safety issue and status lag I personally wouldn't rely on in.

@wfurt
Copy link
Member

wfurt commented Mar 1, 2024

I put up #99181 for the cancellation.

The Connected status on server is actually correct IMHO even if that is not intuitive. When client sends its FIN, the TCP goes to half-closed state. That surfaces on server as 0 length read. But tat that point the server can still write data to the socket. If the client is truly gone it will get RST back (or ICMP) but from TCP state prospective the TCP is not in Closed state. You can use something like netstat on the server to see what OS feels about the socket.

the EOF received on server should be indicative (when non zero buffer is provided) that client will not send any more data. The Connected properly should not be used for that.

@cbworksdev
Copy link

@wfurt Just wanted to add I have also wasted a good amount of time figuring out why the Connected flag goes to false after the cancellation token is cancelled on a ReadAsync. Thanks for #99181 hope it gets merged, although it wont be fast enough for my use case!

@Beowulfe222
Copy link

Just to echo the previous comment, I just ran into the same issue with the Connected flag being set to false when ReadAsync was cancelled via a cancellation token, and until I found this thread it threw me for quite a long debugging detour. This would be great to get fixed, it's incredibly unintuitive.

@wfurt
Copy link
Member

wfurt commented May 11, 2024

The fix for making Socket useable after cancellation was merged. (it will be part of 9.0).
Now I'm not sure if there is anything left here @zahirtezcan-bugs.
I think there are already ways how to solve the problem.

@wfurt wfurt added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 11, 2024
Copy link
Contributor

This issue has been marked needs-author-action and may be missing some important information.

Copy link
Contributor

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

Copy link
Contributor

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@dotnet-policy-service dotnet-policy-service bot removed this from the Future milestone Jun 8, 2024
@karelz karelz added this to the 9.0.0 milestone Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Net.Sockets needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity
Projects
None yet
Development

No branches or pull requests

8 participants