Skip to content

Commit a950b48

Browse files
committed
Address feedback
1 parent 9a04d0b commit a950b48

File tree

1 file changed

+73
-75
lines changed

1 file changed

+73
-75
lines changed

src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.RawSocket.cs

+73-75
Original file line numberDiff line numberDiff line change
@@ -70,21 +70,24 @@ private Socket GetRawSocket(SocketConfig socketConfig)
7070
socket.DualMode = false;
7171
}
7272

73-
if (socketConfig.Options != null && socketConfig.Options.Ttl > 0)
73+
if (socketConfig.Options != null)
7474
{
75-
socket.Ttl = (short)socketConfig.Options.Ttl;
76-
}
77-
78-
if (socketConfig.Options != null && addrFamily == AddressFamily.InterNetwork)
79-
{
80-
if (SendIpHeader)
75+
if (socketConfig.Options.Ttl > 0)
8176
{
82-
// some platforms like OSX don't support DontFragment so we construct IP header instead.
83-
socket.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.HeaderIncluded, 1);
77+
socket.Ttl = (short)socketConfig.Options.Ttl;
8478
}
85-
else
79+
80+
if (addrFamily == AddressFamily.InterNetwork)
8681
{
87-
socket.DontFragment = socketConfig.Options.DontFragment;
82+
if (SendIpHeader)
83+
{
84+
// some platforms like OSX don't support DontFragment so we construct IP header instead.
85+
socket.SetSocketOption(SocketOptionLevel.IP, SocketOptionName.HeaderIncluded, 1);
86+
}
87+
else
88+
{
89+
socket.DontFragment = socketConfig.Options.DontFragment;
90+
}
8891
}
8992
}
9093

@@ -158,41 +161,32 @@ private PingReply SendIcmpEchoRequestOverRawSocket(IPAddress address, byte[] buf
158161
try
159162
{
160163
socket.SendTo(socketConfig.SendBuffer, SocketFlags.None, socketConfig.EndPoint);
161-
}
162-
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
163-
{
164-
return CreateTimedOutPingReply();
165-
}
166164

167-
byte[] receiveBuffer = new byte[MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes + buffer.Length];
165+
byte[] receiveBuffer = new byte[MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes + buffer.Length];
168166

169-
long elapsed;
170-
Stopwatch sw = Stopwatch.StartNew();
171-
// Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response
172-
// to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout.
173-
// For example, when pinging the local host, we need to filter out our own echo requests that the socket reads.
174-
while ((elapsed = sw.ElapsedMilliseconds) < timeout)
175-
{
176-
int bytesReceived;
177-
try
178-
{
179-
bytesReceived = socket.ReceiveFrom(receiveBuffer, SocketFlags.None, ref socketConfig.EndPoint);
180-
}
181-
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
167+
long elapsed;
168+
Stopwatch sw = Stopwatch.StartNew();
169+
// Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response
170+
// to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout.
171+
// For example, when pinging the local host, we need to filter out our own echo requests that the socket reads.
172+
while ((elapsed = sw.ElapsedMilliseconds) < timeout)
182173
{
183-
return CreateTimedOutPingReply();
184-
}
174+
int bytesReceived = socket.ReceiveFrom(receiveBuffer, SocketFlags.None, ref socketConfig.EndPoint);
185175

186-
if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes)
187-
{
188-
continue; // Not enough bytes to reconstruct IP header + ICMP header.
189-
}
176+
if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes)
177+
{
178+
continue; // Not enough bytes to reconstruct IP header + ICMP header.
179+
}
190180

191-
if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, sw, ref ipHeaderLength, out PingReply? reply))
192-
{
193-
return reply;
181+
if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, sw, ref ipHeaderLength, out PingReply? reply))
182+
{
183+
return reply;
184+
}
194185
}
195186
}
187+
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
188+
{
189+
}
196190

197191
// We have exceeded our timeout duration, and no reply has been received.
198192
return CreateTimedOutPingReply();
@@ -205,54 +199,58 @@ private async Task<PingReply> SendIcmpEchoRequestOverRawSocketAsync(IPAddress ad
205199
using (Socket socket = GetRawSocket(socketConfig))
206200
{
207201
int ipHeaderLength = socketConfig.IsIpv4 ? MinIpHeaderLengthInBytes : 0;
202+
CancellationTokenSource timeoutTokenSource = new CancellationTokenSource();
208203

209-
await socket.SendToAsync(
210-
new ArraySegment<byte>(socketConfig.SendBuffer),
211-
SocketFlags.None, socketConfig.EndPoint)
212-
.ConfigureAwait(false);
204+
timeoutTokenSource.CancelAfter(timeout);
213205

214-
byte[] receiveBuffer = new byte[MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes + buffer.Length];
215-
216-
long elapsed;
217-
Stopwatch sw = Stopwatch.StartNew();
218-
// Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response
219-
// to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout.
220-
// For example, when pinging the local host, we need to filter out our own echo requests that the socket reads.
221-
while ((elapsed = sw.ElapsedMilliseconds) < timeout)
206+
try
222207
{
223-
Task<SocketReceiveFromResult> receiveTask = socket.ReceiveFromAsync(
224-
new ArraySegment<byte>(receiveBuffer),
225-
SocketFlags.None,
226-
socketConfig.EndPoint);
227-
228-
try
229-
{
230-
await receiveTask.WaitAsync(TimeSpan.FromMilliseconds(timeout - (int)elapsed)).ConfigureAwait(false);
231-
}
232-
catch (TimeoutException)
208+
await socket.SendToAsync(
209+
new ArraySegment<byte>(socketConfig.SendBuffer),
210+
SocketFlags.None, socketConfig.EndPoint,
211+
timeoutTokenSource.Token)
212+
.ConfigureAwait(false);
213+
214+
byte[] receiveBuffer = new byte[MaxIpHeaderLengthInBytes + IcmpHeaderLengthInBytes + buffer.Length];
215+
216+
Stopwatch sw = Stopwatch.StartNew();
217+
// Read from the socket in a loop. We may receive messages that are not echo replies, or that are not in response
218+
// to the echo request we just sent. We need to filter such messages out, and continue reading until our timeout.
219+
// For example, when pinging the local host, we need to filter out our own echo requests that the socket reads.
220+
while (!timeoutTokenSource.IsCancellationRequested)
233221
{
234-
return CreateTimedOutPingReply();
235-
}
236-
237-
SocketReceiveFromResult receiveResult = receiveTask.GetAwaiter().GetResult();
238-
int bytesReceived = receiveResult.ReceivedBytes;
239-
if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes)
240-
{
241-
continue; // Not enough bytes to reconstruct IP header + ICMP header.
242-
}
243-
244-
if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, sw, ref ipHeaderLength, out PingReply? reply))
245-
{
246-
return reply;
222+
SocketReceiveFromResult receiveResult = await socket.ReceiveFromAsync(
223+
new ArraySegment<byte>(receiveBuffer),
224+
SocketFlags.None,
225+
socketConfig.EndPoint,
226+
timeoutTokenSource.Token)
227+
.ConfigureAwait(false);
228+
229+
int bytesReceived = receiveResult.ReceivedBytes;
230+
if (bytesReceived - ipHeaderLength < IcmpHeaderLengthInBytes)
231+
{
232+
continue; // Not enough bytes to reconstruct IP header + ICMP header.
233+
}
234+
235+
if (TryGetPingReply(socketConfig, receiveBuffer, bytesReceived, sw, ref ipHeaderLength, out PingReply? reply))
236+
{
237+
return reply;
238+
}
247239
}
248240
}
241+
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
242+
{
243+
}
244+
catch (OperationCanceledException)
245+
{
246+
}
249247

250248
// We have exceeded our timeout duration, and no reply has been received.
251249
return CreateTimedOutPingReply();
252250
}
253251
}
254252

255-
private PingReply CreateTimedOutPingReply()
253+
private static PingReply CreateTimedOutPingReply()
256254
{
257255
// Documentation indicates that you should only pay attention to the IPStatus value when
258256
// its value is not "Success", but the rest of these values match that of the Windows implementation.

0 commit comments

Comments
 (0)