Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 6000379

Browse files
benaadamsstephentoub
benaadams
authored and
stephentoub
committed
NetworkChange should capture AsyncLocals (but not to Timer) (#26073)
* NetworkChange shouldn't capture AsyncLocals into its Timer * Capture EC on Linux * Capture EC on OSX * Feedback/dedupe platforms * Better null checking
1 parent 504781c commit 6000379

File tree

6 files changed

+430
-232
lines changed

6 files changed

+430
-232
lines changed

‎src/System.Net.NetworkInformation/src/System.Net.NetworkInformation.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
<Compile Include="System\Net\NetworkInformation\MulticastIPAddressInformationCollection.cs" />
3535
<Compile Include="System\Net\NetworkInformation\NetBiosNodeType.cs" />
3636
<Compile Include="System\Net\NetworkInformation\NetEventSource.NetworkInformation.cs" />
37+
<Compile Include="System\Net\NetworkInformation\NetworkAddressChange.cs" />
3738
<Compile Include="System\Net\NetworkInformation\NetworkAvailabilityEventArgs.cs" />
3839
<Compile Include="System\Net\NetworkInformation\NetworkChangeDelegates.cs" />
3940
<Compile Include="System\Net\NetworkInformation\NetworkInterface.cs" />

‎src/System.Net.NetworkInformation/src/System/Net/NetworkInformation/NetworkAddressChange.Linux.cs‎

Lines changed: 138 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,16 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Collections.Generic;
56
using System.Diagnostics;
6-
using System.Linq;
77
using System.Threading.Tasks;
88
using System.Threading;
99

1010
namespace System.Net.NetworkInformation
1111
{
1212
// Linux implementation of NetworkChange
13-
public class NetworkChange
13+
public partial class NetworkChange
1414
{
15-
private static NetworkAddressChangedEventHandler s_addressChangedSubscribers;
16-
private static NetworkAvailabilityChangedEventHandler s_availabilityChangedSubscribers;
1715
private static volatile int s_socket = 0;
1816
// Lock controlling access to delegate subscriptions, socket initialization, availability-changed state and timer.
1917
private static readonly object s_gate = new object();
@@ -37,30 +35,37 @@ public static event NetworkAddressChangedEventHandler NetworkAddressChanged
3735
{
3836
add
3937
{
40-
lock (s_gate)
38+
if (value != null)
4139
{
42-
if (s_socket == 0)
40+
lock (s_gate)
4341
{
44-
CreateSocket();
45-
}
42+
if (s_socket == 0)
43+
{
44+
CreateSocket();
45+
}
4646

47-
s_addressChangedSubscribers += value;
47+
s_addressChangedSubscribers.TryAdd(value, ExecutionContext.Capture());
48+
}
4849
}
4950
}
5051
remove
5152
{
52-
lock (s_gate)
53+
if (value != null)
5354
{
54-
if (s_addressChangedSubscribers == null && s_availabilityChangedSubscribers == null)
55+
lock (s_gate)
5556
{
56-
Debug.Assert(s_socket == 0, "s_socket != 0, but there are no subscribers to NetworkAddressChanged or NetworkAvailabilityChanged.");
57-
return;
58-
}
57+
if (s_addressChangedSubscribers.Count == 0 && s_availabilityChangedSubscribers.Count == 0)
58+
{
59+
Debug.Assert(s_socket == 0,
60+
"s_socket != 0, but there are no subscribers to NetworkAddressChanged or NetworkAvailabilityChanged.");
61+
return;
62+
}
5963

60-
s_addressChangedSubscribers -= value;
61-
if (s_addressChangedSubscribers == null && s_availabilityChangedSubscribers == null)
62-
{
63-
CloseSocket();
64+
s_addressChangedSubscribers.Remove(value);
65+
if (s_addressChangedSubscribers.Count == 0 && s_availabilityChangedSubscribers.Count == 0)
66+
{
67+
CloseSocket();
68+
}
6469
}
6570
}
6671
}
@@ -70,50 +75,74 @@ public static event NetworkAvailabilityChangedEventHandler NetworkAvailabilityCh
7075
{
7176
add
7277
{
73-
lock (s_gate)
78+
if (value != null)
7479
{
75-
if (s_socket == 0)
76-
{
77-
CreateSocket();
78-
}
79-
if (s_availabilityTimer == null)
80+
lock (s_gate)
8081
{
81-
s_availabilityTimer = new Timer(s_availabilityTimerFiredCallback, null, -1, -1);
82-
}
82+
if (s_socket == 0)
83+
{
84+
CreateSocket();
85+
}
86+
87+
if (s_availabilityTimer == null)
88+
{
89+
// Don't capture the current ExecutionContext and its AsyncLocals onto the timer causing them to live forever
90+
bool restoreFlow = false;
91+
try
92+
{
93+
if (!ExecutionContext.IsFlowSuppressed())
94+
{
95+
ExecutionContext.SuppressFlow();
96+
restoreFlow = true;
97+
}
98+
99+
s_availabilityTimer = new Timer(s_availabilityTimerFiredCallback, null, Timeout.Infinite, Timeout.Infinite);
100+
}
101+
finally
102+
{
103+
// Restore the current ExecutionContext
104+
if (restoreFlow)
105+
ExecutionContext.RestoreFlow();
106+
}
107+
}
83108

84-
s_availabilityChangedSubscribers += value;
109+
s_availabilityChangedSubscribers.TryAdd(value, ExecutionContext.Capture());
110+
}
85111
}
86112
}
87113
remove
88114
{
89-
lock (s_gate)
115+
if (value != null)
90116
{
91-
if (s_addressChangedSubscribers == null && s_availabilityChangedSubscribers == null)
92-
{
93-
Debug.Assert(s_socket == 0, "s_socket != 0, but there are no subscribers to NetworkAddressChanged or NetworkAvailabilityChanged.");
94-
return;
95-
}
96-
97-
s_availabilityChangedSubscribers -= value;
98-
if (s_availabilityChangedSubscribers == null)
117+
lock (s_gate)
99118
{
100-
if (s_availabilityTimer != null)
119+
if (s_addressChangedSubscribers.Count == 0 && s_availabilityChangedSubscribers.Count == 0)
101120
{
102-
s_availabilityTimer.Dispose();
103-
s_availabilityTimer = null;
104-
s_availabilityHasChanged = false;
121+
Debug.Assert(s_socket == 0,
122+
"s_socket != 0, but there are no subscribers to NetworkAddressChanged or NetworkAvailabilityChanged.");
123+
return;
105124
}
106125

107-
if (s_addressChangedSubscribers == null)
126+
s_availabilityChangedSubscribers.Remove(value);
127+
if (s_availabilityChangedSubscribers.Count == 0)
108128
{
109-
CloseSocket();
129+
if (s_availabilityTimer != null)
130+
{
131+
s_availabilityTimer.Dispose();
132+
s_availabilityTimer = null;
133+
s_availabilityHasChanged = false;
134+
}
135+
136+
if (s_addressChangedSubscribers.Count == 0)
137+
{
138+
CloseSocket();
139+
}
110140
}
111141
}
112142
}
113143
}
114144
}
115145

116-
117146
private static void CreateSocket()
118147
{
119148
Debug.Assert(s_socket == 0, "s_socket != 0, must close existing socket before opening another.");
@@ -150,7 +179,7 @@ private static void LoopReadSocket(int socket)
150179
Interop.Sys.ReadEvents(socket, s_networkChangeCallback);
151180
}
152181
}
153-
182+
154183
private static void ProcessEvent(int socket, Interop.Sys.NetworkChangeKind kind)
155184
{
156185
if (kind != Interop.Sys.NetworkChangeKind.None)
@@ -171,7 +200,7 @@ private static void OnSocketEvent(Interop.Sys.NetworkChangeKind kind)
171200
{
172201
case Interop.Sys.NetworkChangeKind.AddressAdded:
173202
case Interop.Sys.NetworkChangeKind.AddressRemoved:
174-
s_addressChangedSubscribers?.Invoke(null, EventArgs.Empty);
203+
OnAddressChanged();
175204
break;
176205
case Interop.Sys.NetworkChangeKind.AvailabilityChanged:
177206
lock (s_gate)
@@ -189,18 +218,77 @@ private static void OnSocketEvent(Interop.Sys.NetworkChangeKind kind)
189218
}
190219
}
191220

221+
private static void OnAddressChanged()
222+
{
223+
Dictionary<NetworkAddressChangedEventHandler, ExecutionContext> addressChangedSubscribers = null;
224+
225+
lock (s_gate)
226+
{
227+
if (s_addressChangedSubscribers.Count > 0)
228+
{
229+
addressChangedSubscribers = new Dictionary<NetworkAddressChangedEventHandler, ExecutionContext>(s_addressChangedSubscribers);
230+
}
231+
}
232+
233+
if (addressChangedSubscribers != null)
234+
{
235+
foreach (KeyValuePair<NetworkAddressChangedEventHandler, ExecutionContext>
236+
subscriber in addressChangedSubscribers)
237+
{
238+
NetworkAddressChangedEventHandler handler = subscriber.Key;
239+
ExecutionContext ec = subscriber.Value;
240+
241+
if (ec == null) // Flow supressed
242+
{
243+
handler(null, EventArgs.Empty);
244+
}
245+
else
246+
{
247+
ExecutionContext.Run(ec, s_runAddressChangedHandler, handler);
248+
}
249+
}
250+
}
251+
}
252+
192253
private static void OnAvailabilityTimerFired(object state)
193254
{
194-
bool changed;
255+
Dictionary<NetworkAvailabilityChangedEventHandler, ExecutionContext> availabilityChangedSubscribers = null;
256+
195257
lock (s_gate)
196258
{
197-
changed = s_availabilityHasChanged;
198-
s_availabilityHasChanged = false;
259+
if (s_availabilityHasChanged)
260+
{
261+
s_availabilityHasChanged = false;
262+
if (s_availabilityChangedSubscribers.Count > 0)
263+
{
264+
availabilityChangedSubscribers =
265+
new Dictionary<NetworkAvailabilityChangedEventHandler, ExecutionContext>(
266+
s_availabilityChangedSubscribers);
267+
}
268+
}
199269
}
200270

201-
if (changed)
271+
if (availabilityChangedSubscribers != null)
202272
{
203-
s_availabilityChangedSubscribers?.Invoke(null, new NetworkAvailabilityEventArgs(NetworkInterface.GetIsNetworkAvailable()));
273+
bool isAvailable = NetworkInterface.GetIsNetworkAvailable();
274+
NetworkAvailabilityEventArgs args = isAvailable ? s_availableEventArgs : s_notAvailableEventArgs;
275+
ContextCallback callbackContext = isAvailable ? s_runHandlerAvailable : s_runHandlerNotAvailable;
276+
277+
foreach (KeyValuePair<NetworkAvailabilityChangedEventHandler, ExecutionContext>
278+
subscriber in availabilityChangedSubscribers)
279+
{
280+
NetworkAvailabilityChangedEventHandler handler = subscriber.Key;
281+
ExecutionContext ec = subscriber.Value;
282+
283+
if (ec == null) // Flow supressed
284+
{
285+
handler(null, args);
286+
}
287+
else
288+
{
289+
ExecutionContext.Run(ec, callbackContext, handler);
290+
}
291+
}
204292
}
205293
}
206294
}

0 commit comments

Comments
 (0)