-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
NetworkChange.NetworkAddressChanged
event leaks threads on Linux
#63788
Comments
Tagging subscribers to this area: @dotnet/ncl Issue DetailsDescriptionWhen you enable HTTP/3 support on Linux and repeatedly get HTTP/2 or HTTP3 web page, ".NET Network Address Change" threads will start to accumulate. It seems network address change event is triggered from https://source.dot.net/#System.Net.Http/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs with call _poolManager.StartMonitoringNetworkChanges(); NetworkAddressChange.Unix.cs will start creating threads with name ".NET Network Address Change": Reproduction Steps.NET 6 console project, source code example:`using System.Diagnostics; static async Task ThreadDiagnostics()
} for (var i = 0; i < 60; i++)
} Dockerfile with installed libmsquic and required tools (procps, curl)#See https://aka.ms/containerfastmode to understand how Visual Studio uses this Dockerfile to build your images for faster debugging. FROM mcr.microsoft.com/dotnet/runtime:6.0-bullseye-slim AS base #install some tools #install libmsquic FROM mcr.microsoft.com/dotnet/sdk:6.0 AS build FROM build AS publish FROM base AS final Expected behavior".NET Network Address Change" should not accumulate, especially if HTTP connections are not pooled.
Actual behavior".NET Network Address Change" threads will start to accumulate and consume resources / memory. Regression?No response Known WorkaroundsNo response ConfigurationNo response Other informationNo response
|
Just small update: HTTP/3 must be enabled in .csproj by adding following entry <ItemGroup>
<RuntimeHostConfigurationOption Include="System.Net.SocketsHttpHandler.Http3Support" Value="true" />
</ItemGroup> |
I just re-ran the repro. It reproduces, but it doesn't seem to be specific to HTTP/3 but rather a leak in This also reproduces with just this, instead of NetworkAddressChangedEventHandler networkChangedDelegate;
networkChangedDelegate = delegate
{
Console.WriteLine("called");
};
NetworkChange.NetworkAddressChanged += networkChangedDelegate;
NetworkChange.NetworkAddressChanged -= networkChangedDelegate; Seems like bug in |
NetworkChange.NetworkAddressChanged
event leaks threads on Linux
Tagging subscribers to this area: @dotnet/ncl Issue DetailsDescriptionWhen you enable HTTP/3 support on Linux and repeatedly get HTTP/2 or HTTP3 web page, ".NET Network Address Change" threads will start to accumulate. It seems network address change event is triggered from https://source.dot.net/#System.Net.Http/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs with call _poolManager.StartMonitoringNetworkChanges(); NetworkAddressChange.Unix.cs will start creating threads with name ".NET Network Address Change": Reproduction Steps.NET 6 console project, source code example:using System.Diagnostics;
static async Task ThreadDiagnostics()
{
var pid = Process.GetCurrentProcess().Id;
var psi = new ProcessStartInfo("ps", $"-T -p {pid}");
psi.RedirectStandardOutput = true;
using var process = Process.Start(psi);
if (process == null)
{
throw new Exception("Could not create process");
}
int threadCountNetworkAddressChange = 0;
var output = await process.StandardOutput.ReadToEndAsync();
using (var sr = new StringReader(output))
{
while (true)
{
var line = await sr.ReadLineAsync();
if (line == null)
{
break;
}
if (line.IndexOf(".NET Network Ad") > 0)
{
threadCountNetworkAddressChange++;
}
//NOTE:
//we are searching for threads containing ".NET Network Ad"
//new Thread(s => LoopReadSocket((int)s!))
//{
// IsBackground = true,
// Name = ".NET Network Address Change"
//}.UnsafeStart(newSocket);
//
//https://source.dot.net/#System.Net.NetworkInformation/System/Net/NetworkInformation/NetworkAddressChange.Unix.cs
}
}
//Console.WriteLine(output);
Console.WriteLine($"{DateTime.Now} '.NET Network Address Change' threads: {threadCountNetworkAddressChange}");
}
for (var i = 0; i < 60; i++)
{
try
{
using (var client = new HttpClient())
{
client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
using var req = new HttpRequestMessage(HttpMethod.Get, "https://cloudflare-quic.com/"); //HTTP/2 or HTTP/3 enabled web server
req.Version = System.Net.HttpVersion.Version20;
using var res = await client.SendAsync(req);
res.EnsureSuccessStatusCode();
}
await ThreadDiagnostics();
}
catch (Exception ex)
{
Console.WriteLine($"Unhandled exception: {ex}");
}
await Task.Delay(1000);
} Dockerfile with installed libmsquic and required tools (procps, curl)
Expected behavior".NET Network Address Change" should not accumulate, especially if HTTP connections are not pooled.
Actual behavior".NET Network Address Change" threads will start to accumulate and consume resources / memory. Regression?No response Known WorkaroundsNo response ConfigurationNo response Other informationNo response
|
Are you saying if you put that in a loop it creates lots of threads that never go away? |
With my test: yes |
I was particularly wondering about @ManickaP's. |
Following Harmony patch seems to workaround this issue :S var harmony = new HarmonyLib.Harmony("MemoryLeakFix");
var t = typeof(System.Net.NetworkInformation.NetworkChange);
{
var method = t.GetMethod("CreateSocket", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
var methodNew = typeof(MyClass).GetMethod("Transpiler", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
harmony.Patch(method, transpiler: new HarmonyLib.HarmonyMethod(methodNew));
}
{
var method = t.GetMethod("CloseSocket", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
var methodNew = typeof(MyClass).GetMethod("Transpiler", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
harmony.Patch(method, transpiler: new HarmonyLib.HarmonyMethod(methodNew));
}
private static IEnumerable<HarmonyLib.CodeInstruction> Transpiler(IEnumerable<HarmonyLib.CodeInstruction> instructions)
{
return new[] { new HarmonyLib.CodeInstruction(System.Reflection.Emit.OpCodes.Ret) };
} |
It does look like there's a race condition between the worker thread and CreateSocket. Should be easy to fix. |
If I try ManickaP example, I get "'.NET Network Address Change' threads: 1" If I try HttpClient example: for (var i = 0; i < 10; i++)
{
try
{
using (var client = new HttpClient())
{
client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
using var req = new HttpRequestMessage(HttpMethod.Get, "https://cloudflare-quic.com/"); //HTTP/2 or HTTP/3 enabled web server
req.Version = System.Net.HttpVersion.Version20;
using var res = await client.SendAsync(req);
res.EnsureSuccessStatusCode();
}
await ThreadDiagnostics();
}
catch (Exception ex)
{
Console.WriteLine($"Unhandled exception: {ex}");
}
await Task.Delay(1000);
} When loop is finished I get '.NET Network Address Change' threads: 10 (also measured few minutes after loop ended) |
Sorry, it is returning now '.NET Network Address Change' threads: 9 for (var i = 0; i < 10; i++)
{
NetworkAddressChangedEventHandler networkChangedDelegate;
networkChangedDelegate = delegate
{
Console.WriteLine("called");
};
NetworkChange.NetworkAddressChanged += networkChangedDelegate;
NetworkChange.NetworkAddressChanged -= networkChangedDelegate;
}
while (true)
{
await ThreadDiagnostics();
await Task.Delay(1000);
} |
With my example I got growing number of threads, even though slowly than with This seems like a race. I also dug a bit around the code and the thread should finish on its own once the Lines 173 to 179 in 0b527a2
The only think I can think of is that the subsequent call of: runtime/src/libraries/Common/src/Interop/Unix/System.Native/Interop.NetworkChange.cs Line 20 in 0b527a2
returns the same fd for the new socket, thus s_socket gets set to the same number even though it's a different run and different registration...
Based on Linux docs: https://man7.org/linux/man-pages/man2/socket.2.html I assume that getting the same fd is highly likely:
|
That was my guess as well. |
Yep, it's always getting the same socket id |
Triage: Used in HTTP/3, we should fix it in 7.0. |
Possible solution: adding "s_socketCreateContext" counter which changes each time CreateSocket() method is called. Method LoopReadSocket then accepts LoopReadSocketArgs args instead of int socket: private static volatile int s_socketCreateContext;
private class LoopReadSocketArgs
{
public LoopReadSocketArgs(int socketCreateContext, int socket)
{
SocketCreateContext = socketCreateContext;
Socket = socket;
}
public int SocketCreateContext { get; }
public int Socket { get; }
}
private static unsafe void CreateSocket()
{
Debug.Assert(s_socket == 0, "s_socket != 0, must close existing socket before opening another.");
int newSocket;
Interop.Error result = Interop.Sys.CreateNetworkChangeListenerSocket(&newSocket);
if (result != Interop.Error.SUCCESS)
{
string message = Interop.Sys.GetLastErrorInfo().GetErrorMessage();
throw new NetworkInformationException(message);
}
s_socket = newSocket;
var socketCreateContext = System.Threading.Interlocked.Increment(ref s_socketCreateContext);
new Thread(s => LoopReadSocket((LoopReadSocketArgs)s!))
{
IsBackground = true,
Name = ".NET Network Address Change"
}.UnsafeStart(new LoopReadSocketArgs(socketCreateContext, newSocket));
}
private static unsafe void LoopReadSocket(LoopReadSocketArgs args)
{
while (args.Socket == s_socket && args.SocketCreateContext == s_socketCreateContext)
{
Interop.Sys.ReadEvents(args.Socket, &ProcessEvent);
}
} |
@vdjuric can you put up a PR for what you suggest? |
Not perfect solution, but I think it should solve thread leaking. I will prepare PR. |
…d event leaks threads on Linux"
…t leaks threads on Linux (#63963) * Trying to fix issue #63788: "NetworkChange.NetworkAddressChanged event leaks threads on Linux" * Adding socket receive timeout to native method + functional test * Update src/native/libs/System.Native/pal_networkchange.c ManickaP replaced tabs with spaces Co-authored-by: Marie Píchová <11718369+ManickaP@users.noreply.github.com> * Using setsockopt after socket was successfully set, checking setsockopt result. * Adding support for conditional network address changed functional test * Adding support for conditional network address changed functional test * Using reference checking method to determine if socket is changed as suggested by user @tmds * Test method update * Test method update due to some failing tests * Removing yield return from test * Reverting back to yield and adding ToArray() when testing if ps is available * Socket wrapper is now nullable * Update src/libraries/System.Net.NetworkInformation/tests/FunctionalTests/NetworkAddressChangedTests.cs Co-authored-by: vdjuric <vladimir@LEGION21.localdomain> Co-authored-by: Marie Píchová <11718369+ManickaP@users.noreply.github.com>
Description
When you enable HTTP/3 support on Linux and repeatedly get HTTP/2 or HTTP3 web page, ".NET Network Address Change" threads will start to accumulate.
It seems network address change event is triggered from https://source.dot.net/#System.Net.Http/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs with call _poolManager.StartMonitoringNetworkChanges();
NetworkAddressChange.Unix.cs will start creating threads with name ".NET Network Address Change":
https://source.dot.net/#System.Net.NetworkInformation/System/Net/NetworkInformation/NetworkAddressChange.Unix.cs
Reproduction Steps
.NET 6 console project, source code example:
Dockerfile with installed libmsquic and required tools (procps, curl)
Expected behavior
".NET Network Address Change" should not accumulate, especially if HTTP connections are not pooled.
Check https://source.dot.net/#System.Net.Http/System/Net/Http/SocketsHttpHandler/HttpConnectionPoolManager.cs
Actual behavior
".NET Network Address Change" threads will start to accumulate and consume resources / memory.
Regression?
No response
Known Workarounds
No response
Configuration
No response
Other information
No response
The text was updated successfully, but these errors were encountered: