Skip to content

UdpClient with span support #53429

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.

Sign up for GitHub

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

Merged
antonfirsov merged 9 commits into from
Jun 24, 2021
Merged

UdpClient with span support #53429

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f05bfbf
Initial commit: Send() with span.
lateapexearlyspeed May 28, 2021
e5cd921
Add other send methods; refactor code by reusing GetEndpoint() for (h…
lateapexearlyspeed Jun 1, 2021
32050da
Add ReceiveAsync().
lateapexearlyspeed Jun 3, 2021
d9c5e2b
Add method 'SendAsync(System.ReadOnlyMemory<byte> datagram, string? h…
lateapexearlyspeed Jun 4, 2021
d8e144c
Fix comment: Add xml doc for new APIs.
lateapexearlyspeed Jun 11, 2021
05b0cf2
Fix comment: Refine test codes.
lateapexearlyspeed Jun 15, 2021
14465d0
Remove tab.
lateapexearlyspeed Jun 15, 2021
0ca0fee
Use ThrowsAnyAsync() to assert.
lateapexearlyspeed Jun 17, 2021
a368fff
Fix comment: add more test for new SendAsync overloads.
lateapexearlyspeed Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/libraries/System.Net.Sockets/ref/System.Net.Sockets.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -757,10 +757,15 @@ public void JoinMulticastGroup(System.Net.IPAddress multicastAddr, System.Net.IP
public byte[] Receive([System.Diagnostics.CodeAnalysis.NotNullAttribute] ref System.Net.IPEndPoint? remoteEP) { throw null; }
public System.Threading.Tasks.Task<System.Net.Sockets.UdpReceiveResult> ReceiveAsync() { throw null; }
public int Send(byte[] dgram, int bytes) { throw null; }
public int Send(System.ReadOnlySpan<byte> datagram) {throw null; }
public int Send(byte[] dgram, int bytes, System.Net.IPEndPoint? endPoint) { throw null; }
public int Send(System.ReadOnlySpan<byte> datagram, System.Net.IPEndPoint? endPoint) { throw null; }
Comment thread
stephentoub marked this conversation as resolved.
public int Send(byte[] dgram, int bytes, string? hostname, int port) { throw null; }
public int Send(System.ReadOnlySpan<byte> datagram, string? hostname, int port) { throw null; }
public System.Threading.Tasks.Task<int> SendAsync(byte[] datagram, int bytes) { throw null; }
public System.Threading.Tasks.ValueTask<int> SendAsync(System.ReadOnlyMemory<byte> datagram, System.Threading.CancellationToken cancellationToken = default) { throw null; }
public System.Threading.Tasks.Task<int> SendAsync(byte[] datagram, int bytes, System.Net.IPEndPoint? endPoint) { throw null; }
public System.Threading.Tasks.ValueTask<int> SendAsync(System.ReadOnlyMemory<byte> datagram, System.Net.IPEndPoint? endPoint, System.Threading.CancellationToken cancellationToken = default) { throw null; }
public System.Threading.Tasks.Task<int> SendAsync(byte[] datagram, int bytes, string? hostname, int port) { throw null; }
}
public partial struct UdpReceiveResult : System.IEquatable<System.Net.Sockets.UdpReceiveResult>
Expand Down
81 changes: 56 additions & 25 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/UDPClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using System.Runtime.Versioning;
using System.Threading;

namespace System.Net.Sockets
{
Expand Down Expand Up @@ -334,6 +335,17 @@ private void ValidateDatagram(byte[] datagram, int bytes, IPEndPoint? endPoint)
}
}

private void ValidateDatagram(ReadOnlyMemory<byte> datagram, IPEndPoint? endPoint)
Copy link
Copy Markdown
Contributor

@antonfirsov antonfirsov Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The datagram argument is unused, note that this also makes the naming unfortunate. Since this particular overload has only one usage, it's cleaner to inline probably.

Copy link
Copy Markdown
Contributor Author

@lateapexearlyspeed lateapexearlyspeed Jun 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored code to validate inside calling code.

{
ThrowIfDisposed();

if (_active && endPoint != null)
{
// Do not allow sending packets to arbitrary host when connected.
throw new InvalidOperationException(SR.net_udpconnected);
}
}

private IPEndPoint? GetEndpoint(string? hostname, int port)
{
if (_active && ((hostname != null) || (port != 0)))
Expand Down Expand Up @@ -600,6 +612,9 @@ public void DropMulticastGroup(IPAddress multicastAddr, int ifindex)
public Task<int> SendAsync(byte[] datagram, int bytes) =>
SendAsync(datagram, bytes, null);

public ValueTask<int> SendAsync(ReadOnlyMemory<byte> datagram, CancellationToken cancellationToken = default) =>
Copy link
Copy Markdown
Contributor

@antonfirsov antonfirsov Jun 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to a new rule, we need to add triple-slash docs for all new public methods right in product code PR-s. They will be fed to a tool to generate API docs.

You can find examples in Socket.cs. My recommendation is to copy and alter documentation text from existing overloads

Copy link
Copy Markdown
Contributor Author

@lateapexearlyspeed lateapexearlyspeed Jun 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added xml doc for new APIs.

Comment on lines +615 to +616
Copy link
Copy Markdown
Contributor

@antonfirsov antonfirsov Jun 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is makes the bar really unfair for community contributors IMO, but I think we should also copy the Remarks sections when present:

https://github.com/dotnet/dotnet-api-docs/blob/f6609e53f05d5c00724aaaf5a31120aa5014b79b/xml/System.Net.Sockets/UdpClient.xml#L2324-L2338

(This applies to all new overloads.)

@carlossanlop any plans to make this process simpler and the code less bloated with long copy-pasted blocks? (Not sure if you are the right person to tag about this.)

Copy link
Copy Markdown
Contributor

@geoffkizer geoffkizer Jun 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long-term plan is to merge the existing external docs back in to the code, so that the code is the one source of truth.

I don't know when/how this will happen; @carlossanlop is the right person to talk to.

SendAsync(datagram, null, cancellationToken);

public Task<int> SendAsync(byte[] datagram, int bytes, string? hostname, int port) =>
SendAsync(datagram, bytes, GetEndpoint(hostname, port));

Expand All @@ -618,6 +633,21 @@ public Task<int> SendAsync(byte[] datagram, int bytes, IPEndPoint? endPoint)
}
}

public ValueTask<int> SendAsync(ReadOnlyMemory<byte> datagram, IPEndPoint? endPoint, CancellationToken cancellationToken = default)
{
ValidateDatagram(datagram, endPoint);

if (endPoint is null)
{
return _clientSocket.SendAsync(datagram, SocketFlags.None, cancellationToken);
}
else
{
CheckForBroadcast(endPoint.Address);
return _clientSocket.SendToAsync(datagram, SocketFlags.None, endPoint, cancellationToken);
}
}

public Task<UdpReceiveResult> ReceiveAsync()
{
ThrowIfDisposed();
Expand Down Expand Up @@ -892,45 +922,32 @@ public int Send(byte[] dgram, int bytes, IPEndPoint? endPoint)
return Client.SendTo(dgram, 0, bytes, SocketFlags.None, endPoint);
}


// Sends a UDP datagram to the specified port on the specified remote host.
public int Send(byte[] dgram, int bytes, string? hostname, int port)
// Sends a UDP datagram to the host at the remote end point.
public int Send(ReadOnlySpan<byte> datagram, IPEndPoint? endPoint)
{
ThrowIfDisposed();

if (dgram == null)
{
throw new ArgumentNullException(nameof(dgram));
}
if (_active && ((hostname != null) || (port != 0)))
if (_active && endPoint != null)
{
// Do not allow sending packets to arbitrary host when connected
throw new InvalidOperationException(SR.net_udpconnected);
}

if (hostname == null || port == 0)
{
return Client.Send(dgram, 0, bytes, SocketFlags.None);
}

IPAddress[] addresses = Dns.GetHostAddresses(hostname);

int i = 0;
for (; i < addresses.Length && !IsAddressFamilyCompatible(addresses[i].AddressFamily); i++)
if (endPoint == null)
{
; // just count the addresses
return Client.Send(datagram, SocketFlags.None);
}

if (addresses.Length == 0 || i == addresses.Length)
{
throw new ArgumentException(SR.net_invalidAddressList, nameof(hostname));
}
CheckForBroadcast(endPoint.Address);

CheckForBroadcast(addresses[i]);
IPEndPoint ipEndPoint = new IPEndPoint(addresses[i], port);
return Client.SendTo(dgram, 0, bytes, SocketFlags.None, ipEndPoint);
return Client.SendTo(datagram, SocketFlags.None, endPoint);
}

// Sends a UDP datagram to the specified port on the specified remote host.
public int Send(byte[] dgram, int bytes, string? hostname, int port) => Send(dgram, bytes, GetEndpoint(hostname, port));

// Sends a UDP datagram to the specified port on the specified remote host.
public int Send(ReadOnlySpan<byte> datagram, string? hostname, int port) => Send(datagram, GetEndpoint(hostname, port));

// Sends a UDP datagram to a remote host.
public int Send(byte[] dgram, int bytes)
Expand All @@ -950,6 +967,20 @@ public int Send(byte[] dgram, int bytes)
return Client.Send(dgram, 0, bytes, SocketFlags.None);
}

// Sends a UDP datagram to a remote host.
public int Send(ReadOnlySpan<byte> datagram)
{
ThrowIfDisposed();

if (!_active)
{
// only allowed on connected socket
throw new InvalidOperationException(SR.net_notconnected);
}

return Client.Send(datagram, SocketFlags.None);
}

private void ThrowIfDisposed()
{
if (_disposed)
Expand Down