dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.36k stars 4.75k forks source link

Socket.InternalShutdown is ignoring LingerOption #104000

Open kaiohenrique opened 4 months ago

kaiohenrique commented 4 months ago

Description

We have set LingerOption to avoid TIME_WAIT socket state but it seems not change the behavior. Extending NetworkStream and calling socket Close and Dispose seems to be doing the trick. No sure why Socket.InternalShutdown is impedind the LingerOption to work as expected. Am I missing anything here? Note: in the the request headers, we are sending "Connect: close" to avoid pooling

Reproduction Steps

SocketsHttpHandler ConnectCallback overwrite

 ConnectCallback = async (context, cts) =>
 {
     var socket = new Socket(SocketType.Stream, ProtocolType.Tcp)
     {
         NoDelay = true,
         LingerState = new LingerOption(true, 0)
     };

     try
     {                                
         await socket.ConnectAsync(context.DnsEndPoint, cts);
         return new OverrideNetworkStream(socket);
     }
     catch (Exception e)
     {
         socket.Dispose();
         throw;
     }

 }

OverrideNetworkStream override the NetworkStream.Dispose as a wokaround because Socket.InternalShutdown seems to be causing the LingerOptions to not work as expected

 public class OverrideNetworkStream : NetworkStream
 {        
     private Socket _socket;        
     public OverrideNetworkStream(Socket socket) : base(socket, false)
     {            
         _socket = socket;            
     }
     private  object call(object o, string methodName, params object[] args)
     {
         var mi = o.GetType().GetMethod(methodName, System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
         if (mi != null)
         {
             return mi.Invoke(o, args);
         }
         return null;
     }

     protected override void Dispose(bool disposing)
     {
         //call(_socket, "InternalShutdown", SocketShutdown.Both); to reproduce the TIME_WAIT you should not comment this line

         _socket.Close();
         _socket.Dispose();
         base.Dispose(disposing);                    
     }
 }

Expected behavior

NetworkStream should respect LingerOptions

Actual behavior

As NetworkStream is calling Socket.InternalShutdown, LingerOption is not being respected and socket is leaking in TIME_WAIT state

Regression?

Only tested in .Net8 both in Windows and Linux

Known Workarounds

No response

Configuration

No response

Other information

No response

dotnet-policy-service[bot] commented 4 months ago

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

wfurt commented 4 months ago

Why do you set ownsSocket: false? AFAIK the behavior can change based on the ownership. cc @tmds

kaiohenrique commented 4 months ago

Hi, @wfurt In this case, I want to avoid the NetworkStream.Dispose to execute the Socket.InternalShutdown method. Overriding the NetworkStream.Dispose method with ownsSocket: false and only performing the Socket.Close and Socket.Dispose seems to workaround the issue and LingerOption is correctly applied. Is that a valid workaround?

kaiohenrique commented 4 months ago

Hi, @wfurt @tmds I need help to understand if avoiding Socket.InternalShutdown is a valid workaround so LingerOption works as expected. In our tests, avoiding the call to Socket.InternalShutdownm with LingerOptions(true, 0) got rid of connections in time_wait state completly. Are we missing any side effect?

Consider that requests has Connection:close to avoid connection pooling, a customer requeriment

kaiohenrique commented 4 months ago

Update: I have seen this behavior LingerOption(true,0)

_socket.Shutdown(SocketShutdown.Receive); //avoid time_wait and LingerOption is respected _socket.Shutdown(SocketShutdown.Send); //lots of sockets in time_wait _socket.Shutdown(SocketShutdown.Both); //lots of sockets in time_wait _socket.Close(); _socket.Dispose();

liveans commented 4 months ago

Triage: Not critical for 9.0. Moving to the future.

tmds commented 4 months ago

I can't reproduce this on Linux (Fedora 39).

I'm using this code to try to reproduce the issue:

string host = args.Length > 0 ? args[0] : "www.microsoft.com";

for (int i = 0; i < 10; i++)
{
    Socket clientSocket = new Socket(SocketType.Stream, ProtocolType.Tcp)
    {
        NoDelay = true,
        LingerState = new LingerOption(true, 0)
    };

    Console.WriteLine($"Connecting to '{host}'");
    await clientSocket.ConnectAsync(new DnsEndPoint(host, 80));
    System.Console.WriteLine($"Connect succeeded from {clientSocket.LocalEndPoint}!");

    NetworkStream ns = new NetworkStream(clientSocket, ownsSocket: true);
    ns.Dispose();
    System.Console.WriteLine($"Disposed.");
}

When I comment out the LingerState = new LingerOption(true, 0) line, I get sockets in the TIME_WAIT state when the program is done. When I keep the line, they are gone.

kaiohenrique commented 4 months ago

Hi, @tmds Are you able to reproduce adding a Task.Delay after the loop?

tmds commented 2 months ago

Are you able to reproduce adding a Task.Delay after the loop?

No, also when the program doesn't exit, setting the new LingerOption(true, 0) option results in there being no TIME_WAIT sockets.

I can't reproduce the issue (on Fedora Linux).