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 cache for Socket.LocalEndPoint #39313

Merged
merged 28 commits into from
Sep 2, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2e977ea
Add cache for LocalEndPoint
CarnaViire Jul 14, 2020
579f788
Add RightEndPoint property, clear _localEndPoint in RightEndPoint setter
CarnaViire Jul 14, 2020
bc08140
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Jul 14, 2020
c68e608
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Jul 20, 2020
3771a6d
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Jul 23, 2020
d953b0f
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Jul 23, 2020
f0a5ec9
Roll back property changes
CarnaViire Jul 23, 2020
999d596
Fix _nonBlockingConnectRightEndPoint
CarnaViire Jul 23, 2020
79b7153
Merge branch 'master' into issue-1482
CarnaViire Jul 31, 2020
d715a31
Add clear on error
CarnaViire Aug 4, 2020
c3eb4ec
Add clear on connect and tests
CarnaViire Aug 5, 2020
324e060
Add caching test
CarnaViire Aug 5, 2020
283f11b
Use BindToAnonymousPort
CarnaViire Aug 11, 2020
0a6a26a
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 11, 2020
1ff3e14
Fix typo
CarnaViire Aug 11, 2020
40f872e
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 11, 2020
09b42c0
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 12, 2020
24e1b66
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 18, 2020
ec8e54e
Assign _localEndPoint from the listener
CarnaViire Aug 18, 2020
3d9385b
tmp commit to check on mac
CarnaViire Aug 24, 2020
4e183ed
tmp
CarnaViire Aug 24, 2020
e626406
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 24, 2020
74988a2
Merge branch 'master' into issue-1482
CarnaViire Aug 25, 2020
eeb057d
Fix _localEndPoint clearing on mac
CarnaViire Aug 25, 2020
85de50d
Inline wildcard addresses
CarnaViire Aug 26, 2020
b86cc93
Merge remote-tracking branch 'upstream/master' into issue-1482
CarnaViire Aug 26, 2020
cbaed88
PR fixes
CarnaViire Aug 27, 2020
d0fa759
Rewrite tests with SocketTestHelperBase
CarnaViire Sep 1, 2020
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
74 changes: 39 additions & 35 deletions src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,22 @@ public partial class Socket : IDisposable
{
internal const int DefaultCloseTimeout = -1; // NOTE: changing this default is a breaking change.

private static readonly HashSet<IPAddress> s_wildcardAddresses = new HashSet<IPAddress>
antonfirsov marked this conversation as resolved.
Show resolved Hide resolved
{
IPAddress.Any, IPAddress.IPv6Any, IPAddress.Any.MapToIPv6()
};

private SafeSocketHandle _handle;

// _rightEndPoint is null if the socket has not been bound. Otherwise, it is any EndPoint of the
// correct type (IPEndPoint, etc).
internal EndPoint? _rightEndPoint;
private EndPoint? _localEndPoint; // Cached LocalEndPoint value. Will clear on connect, error and disconnect
internal EndPoint? _remoteEndPoint;

// Cached LocalEndPoint value. Cleared on disconnect and error. Cached wildcard addresses are
// also cleared on connect and accept.
private EndPoint? _localEndPoint;

// These flags monitor if the socket was ever connected at any time and if it still is.
private bool _isConnected;
private bool _isDisconnected;
Expand Down Expand Up @@ -216,7 +224,7 @@ private unsafe Socket(SafeSocketHandle handle, bool loadPropertiesFromHandle)
}

_isConnected = true;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
antonfirsov marked this conversation as resolved.
Show resolved Hide resolved
break;

case SocketError.InvalidArgument:
Expand All @@ -226,7 +234,7 @@ private unsafe Socket(SafeSocketHandle handle, bool loadPropertiesFromHandle)
// whether we're actually connected or not, err on the side of saying
// we're connected.
_isConnected = true;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
antonfirsov marked this conversation as resolved.
Show resolved Hide resolved
break;
}
}
Expand Down Expand Up @@ -320,7 +328,7 @@ public int Available
// Update the state if we've become connected after a non-blocking connect.
_isConnected = true;
_rightEndPoint = _nonBlockingConnectRightEndPoint;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
_nonBlockingConnectInProgress = false;
}

Expand All @@ -329,7 +337,7 @@ public int Available
return null;
}

//if (_localEndPoint == null)
if (_localEndPoint == null)
{
Internals.SocketAddress socketAddress = IPEndPointExtensions.Serialize(_rightEndPoint);

Expand Down Expand Up @@ -367,7 +375,7 @@ public int Available
// Update the state if we've become connected after a non-blocking connect.
_isConnected = true;
_rightEndPoint = _nonBlockingConnectRightEndPoint;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
_nonBlockingConnectInProgress = false;
}

Expand Down Expand Up @@ -479,7 +487,7 @@ public bool Connected
// Update the state if we've become connected after a non-blocking connect.
_isConnected = true;
_rightEndPoint = _nonBlockingConnectRightEndPoint;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
_nonBlockingConnectInProgress = false;
}

Expand Down Expand Up @@ -4871,7 +4879,12 @@ internal Socket UpdateAcceptSocket(Socket socket, EndPoint remoteEP)
socket._protocolType = _protocolType;
socket._rightEndPoint = _rightEndPoint;
socket._remoteEndPoint = remoteEP;
socket._localEndPoint = _localEndPoint;

// If the listener socket was bound to a wildcard address, then the `accept` system call
// will assign a specific address to the accept socket's local endpoint instead of a
// wildcard address. In that case we should not copy listener's wildcard local endpoint.

socket._localEndPoint = !IsWildcardEndPoint(_localEndPoint) ? _localEndPoint : null;

// The socket is connected.
socket.SetToConnected();
Expand Down Expand Up @@ -4904,44 +4917,35 @@ internal void SetToConnected()
// some point in time update the perf counter as well.
_isConnected = true;
_isDisconnected = false;
HandleLocalEndPointOnConnect();
UpdateLocalEndPointOnConnect();
if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "now connected");
}

private void HandleLocalEndPointOnConnect()
private void UpdateLocalEndPointOnConnect()
{
//_localEndPoint = null;
if (_localEndPoint == null)
{
return;
}
// If the client socket was bound to a wildcard address, then the `connect` system call
// will assign a specific address to the client socket's local endpoint instead of a
// wildcard address. In that case we should clear the cached wildcard local endpoint.

if (_localEndPoint is IPEndPoint ipLocalEndpoint)
if (IsWildcardEndPoint(_localEndPoint))
{
// If a client socket was bound to a wildcard address, then the `connect` system call
// will assign a specific address to the client socket's local endpoint instead of a
// wildcard address. In that case we should clear the cached wildcard local endpoint.

// If a listener socket was bound to a wildcard address, then the `accept` system call
// will assign a specific address the accept socket's local endpoint instead of a
// wildcard address. In that case we should clear the accept socket's cached wildcard
// local endpoint copied from listener.

if (IsWildcardAddress(ipLocalEndpoint.Address))
{
_localEndPoint = null;
}
_localEndPoint = null;
}
}

private bool IsWildcardAddress(IPAddress address)
private bool IsWildcardEndPoint(EndPoint? endPoint)
{
return true;
/*return address.ToString() == IPAddress.Any.ToString()
|| address.ToString() == IPAddress.IPv6Any.ToString()
|| address.ToString() == IPAddress.Any.MapToIPv6().ToString();*/
if (endPoint == null)
{
return false;
}

//return address == IPAddress.Any || address == IPAddress.IPv6Any || address == IPAddress.Any.MapToIPv6();
if (endPoint is IPEndPoint ipEndpoint)
{
return s_wildcardAddresses.Contains(ipEndpoint.Address);
}

return false;
}

internal void SetToDisconnected()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public async Task TcpSocket_ServerBoundToWildcardAddress_AcceptSocketLocalEPIsSp

Socket accept = await acceptTask;
Assert.Equal(accept.RemoteEndPoint, client.LocalEndPoint);
Assert.Equal(accept.LocalEndPoint, client.RemoteEndPoint);

Assert.Equal(Wildcard, GetLocalEPAddress(server)); // server -> stays as wildcard
Assert.Equal(Loopback, GetLocalEPAddress(accept)); // accept -> specific
Expand Down Expand Up @@ -202,7 +203,7 @@ public async Task TcpSocket_ServerBoundToSpecificAddress_AcceptSocketLocalEPIsSa
}
}

//[Fact]
[Fact]
public void LocalEndPoint_IsCached()
{
using (Socket socket = CreateTcpSocket())
Expand Down