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

Add CancellationToken overloads to Socket.ConnectAsync and Socket.AcceptAsync #921

Closed
tmds opened this issue Apr 23, 2019 · 10 comments
Closed
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets
Milestone

Comments

@tmds
Copy link
Member

tmds commented Apr 23, 2019

CancellationTokens were added to SendAsync and ReceiveAsync in PR dotnet/corefx#36516

This for adding overloads to ConnectAsync and AcceptAsync:

class SocketTaskExtensions
{
   public static ValueTask ConnectAsync (this Socket socket, EndPoint remoteEP, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, IPAddress address, int port, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, IPAddress[] addresses, int port, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, string host, int port, CancellationToken cancellationToken);
   public static ValueTask<Socket> AcceptAsync (this Socket socket, CancellationToken cancellationToken);
   public static ValueTask<Socket> AcceptAsync (this Socket socket, Socket acceptSocket, CancellationToken cancellationToken);

   // existing methods:
   // public static Task ConnectAsync (this Socket socket, EndPoint remoteEP);
   // public static Task ConnectAsync (this Socket socket, IPAddress address, int port);
   // public static Task ConnectAsync (this Socket socket, IPAddress[] addresses, int port);
   // public static Task ConnectAsync (this Socket socket, string host, int port);
   // public static Task<Socket> AcceptAsync (this Socket socket);
   // public static Task<Socket> AcceptAsync (this Socket socket, Socket acceptSocket);
}

class Socket
{
   public static void CancelAcceptAsync(SocketAsyncEventArgs e);

   // existing methods:
   // public bool AcceptAsync (SocketAsyncEventArgs e);
   // public bool ConnectAsync (SocketAsyncEventArgs e);
   // public static void CancelConnectAsync (SocketAsyncEventArgs e);
}

class TcpClient
{
   public ValueTask ConnectAsync (string host, int port, CancellationToken cancellationToken);
   public ValueTask ConnectAsync (IPAddress address, int port, CancellationToken cancellationToken);
   public ValueTask ConnectAsync (IPAddress[] addresses, int port, CancellationToken cancellationToken);

   // existing methods:
   // public Task ConnectAsync (string host, int port);
   // public Task ConnectAsync (IPAddress address, int port);
   // public Task ConnectAsync (IPAddress[] addresses, int port);
}

class TcpListener
{
   public ValueTask<Socket> AcceptSocketAsync (CancellationToken cancellationToken);
   public ValueTask<TcpClient> AcceptTcpClientAsync (CancellationToken cancellationToken);

   // existing methods:
   // public Task<Socket> AcceptSocketAsync ();
   // public Task<TcpClient> AcceptTcpClientAsync ();
}

CC @stephentoub

@stephentoub
Copy link
Member

CancellationTokens were added to SendAsync and ReceiveAsync in dotnet/corefx#36516

FWIW, they were actually added in .NET Core 2.1, but the tokens were only checked prior to initiating the call. dotnet/corefx#36516 plumbed them all the way down.

@tmds
Copy link
Member Author

tmds commented Apr 23, 2019

ah ok.

I took a look what is possible with current APIs:
A ConnectAsync(SocketAsyncEventArgs) can be canceled using CancelConnectAsync(SocketAsyncEventArgs).
There is no API to cancel an AcceptAsync.

@clairernovotny
Copy link

clairernovotny commented May 29, 2019

Does this include UdpClient? I just looked at the surface area for the type in NET Standard 2.1 and did not see a SendAsync or RecieveAsync method with a CancellationToken overload. It would be useful there too.

@tmds
Copy link
Member Author

tmds commented Jun 3, 2019

Does this include UdpClient? I just looked at the surface area for the type in NET Standard 2.1 and did not see a SendAsync or RecieveAsync method with a CancellationToken overload. It would be useful there too.

Those are not included (currently). Adding them to UdpClient is mostly for convenience. You can use the CancellationToken overloads via the UdpClient.Client (which gives you the Socket).

@karelz
Copy link
Member

karelz commented Sep 10, 2019

TODO: Looks also into other APIs on Sockets as mentioned in #19592

@maryamariyan maryamariyan transferred this issue from dotnet/corefx Dec 16, 2019
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net.Sockets untriaged New issue has not been triaged by the area owner labels Dec 16, 2019
@maryamariyan maryamariyan added this to the 5.0 milestone Dec 16, 2019
@tmds
Copy link
Member Author

tmds commented Dec 17, 2019

public static ValueTask<Socket> AcceptAsync (this Socket socket, Socket acceptSocket, CancellationToken cancellationToken);

This one will throw PNSE on non-Windows:

// AcceptSocket is not supported on Unix.
if (acceptSocket != null)
{
throw new PlatformNotSupportedException(SR.PlatformNotSupported_AcceptSocket);
}

@xsacha
Copy link

xsacha commented Jan 4, 2020

Slightly unrelated: Does this mean AcceptTcpClientAsync (as a layer on top of Socket) won't work unless we use Windows? What is the alternative?

@tmds
Copy link
Member Author

tmds commented Jan 6, 2020

@xsacha only the overloads that pass in an existing socket as an acceptSocket argument don't work on non-Windows. AcceptTcpClientAsync doesn't do that.

@karelz karelz removed the untriaged New issue has not been triaged by the area owner label Jan 7, 2020
@terrajobst terrajobst added the blocked Issue/PR is blocked on something - see comments label Feb 11, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review labels Feb 20, 2020
@terrajobst
Copy link
Contributor

terrajobst commented Feb 20, 2020

Video

  • We should file an issue to cover the remaining APIs ReceiveAsync, ReceiveFromAsync, ReceiveMessageFromAsync, SendToAsync (@scalablecory)
  • Otherwise looks good
class SocketTaskExtensions
{
   public static ValueTask ConnectAsync (this Socket socket, EndPoint remoteEP, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, IPAddress address, int port, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, IPAddress[] addresses, int port, CancellationToken cancellationToken);
   public static ValueTask ConnectAsync (this Socket socket, string host, int port, CancellationToken cancellationToken);
   public static ValueTask<Socket> AcceptAsync (this Socket socket, CancellationToken cancellationToken);
   public static ValueTask<Socket> AcceptAsync (this Socket socket, Socket acceptSocket, CancellationToken cancellationToken);

   // existing methods:
   // public static Task ConnectAsync (this Socket socket, EndPoint remoteEP);
   // public static Task ConnectAsync (this Socket socket, IPAddress address, int port);
   // public static Task ConnectAsync (this Socket socket, IPAddress[] addresses, int port);
   // public static Task ConnectAsync (this Socket socket, string host, int port);
   // public static Task<Socket> AcceptAsync (this Socket socket);
   // public static Task<Socket> AcceptAsync (this Socket socket, Socket acceptSocket);
}

class Socket
{
   public static void CancelAcceptAsync(SocketAsyncEventArgs e);

   // existing methods:
   // public bool AcceptAsync (SocketAsyncEventArgs e);
   // public bool ConnectAsync (SocketAsyncEventArgs e);
   // public static void CancelConnectAsync (SocketAsyncEventArgs e);
}

class TcpClient
{
   public ValueTask ConnectAsync (string host, int port, CancellationToken cancellationToken);
   public ValueTask ConnectAsync (IPAddress address, int port, CancellationToken cancellationToken);
   public ValueTask ConnectAsync (IPAddress[] addresses, int port, CancellationToken cancellationToken);

   // existing methods:
   // public Task ConnectAsync (string host, int port);
   // public Task ConnectAsync (IPAddress address, int port);
   // public Task ConnectAsync (IPAddress[] addresses, int port);
}

class TcpListener
{
   public ValueTask<Socket> AcceptSocketAsync (CancellationToken cancellationToken);
   public ValueTask<TcpClient> AcceptTcpClientAsync (CancellationToken cancellationToken);

   // existing methods:
   // public Task<Socket> AcceptSocketAsync ();
   // public Task<TcpClient> AcceptTcpClientAsync ();
}

@stephentoub stephentoub removed the blocked Issue/PR is blocked on something - see comments label Mar 2, 2020
@scalablecory
Copy link
Contributor

Closing for mega-issue #33418.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Net.Sockets
Projects
None yet
Development

No branches or pull requests

9 participants