-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Endless HttpMessageHandler cleanup cycle with C# cmdlets using .NET generic host #36172
Comments
Tagging subscribers to this area: @dotnet/ncl |
Do you have a small repro demonstrating this? |
Thanks very much for the comment @scalablecory . The repo where this problem occus is actually https://github.com/markm77/open-banking-connector-csharp - the PS module project is https://github.com/markm77/open-banking-connector-csharp/tree/master/src/OpenBankingConnector.Configuration. This project is very much still in development but I did a big effort to create the smallest possible repro today including creating a no-action cmdlet. However this didn't work - seems a lot of the complexity is needed for a repro. I will give creating a smallish repro another go later in the week but just to be clear the problem is now fully reproducible with the full project. After two minutes exactly the following messages occur:
Is there any way I can reduce this 120 s to help me create a smallish repro? Just means I don't have to wait 2 mins every time I try something..... |
Okay, sorry for the delay with this as creating a minimal repro from a large codebase turned out to be very time-consuming esp. with the 2min test time for each change. However I've done it now so hopefully this can be finally fixed. You can repro by creating a C# project with two source files as follows. Project file <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.1</TargetFramework>
<RuntimeIdentifier>win10-x64</RuntimeIdentifier>
<AssemblyName>PSModuleIssueRepro</AssemblyName>
<RootNamespace>PSModuleIssueRepro</RootNamespace>
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Hosting" Version="3.1.5" />
<PackageReference Include="Microsoft.Extensions.Http" Version="3.1.5" />
<PackageReference Include="PowerShellStandard.Library" Version="5.1.0" PrivateAssets="all">
<PrivateAssets>All</PrivateAssets>
</PackageReference>
</ItemGroup>
</Project> Cmdlet file using System.Management.Automation;
using System.Net.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
namespace PSModuleIssueRepro
{
[Cmdlet(VerbsCommon.New, "Test")]
[OutputType(typeof(void))]
public class NewTest : Cmdlet
{
protected readonly IHost _host;
public NewTest()
{
var builder = new HostBuilder()
.ConfigureServices(
(hostContext, services) =>
{
services.AddHttpClient("client1");
services.AddSingleton<IService1>(
sp =>
{
var hcf = sp.GetRequiredService<IHttpClientFactory>();
var client = hcf.CreateClient("client1");
return new Service1(client);
});
})
.UseConsoleLifetime()
.ConfigureLogging(
(hostingContext, logging) =>
{
logging.SetMinimumLevel(LogLevel.Debug);
logging.AddConsole();
});
_host = builder.Build();
_host.Start();
}
protected override void BeginProcessing()
{
WriteVerbose($"start");
}
protected override void ProcessRecord()
{
using var serviceScope = _host.Services.CreateScope();
var _ = serviceScope.ServiceProvider.GetService<IService1>();
}
protected override void EndProcessing()
{
WriteVerbose($"end");
}
}
} Service file using System.Net.Http;
namespace PSModuleIssueRepro
{
public interface IService1 { }
public class Service1 : IService1
{
private readonly HttpClient _httpClient;
public Service1(HttpClient httpClient)
{
_httpClient = httpClient;
}
}
} After creating and building the project, you can then reproduce the issue by launching PowerShell 7 via Import-Module .\PSModuleIssueRepro\bin\Debug\netstandard2.1\win10-x64\PSModuleIssueRepro.dll
New-Test You will find after exactly 2 minutes you get error messages as follows and you will need to force-exit to exit the PowerShell shell (
Hopefully this is sufficient info to repro the problem, feel free to ask any follow-up questions etc.... |
I believe the logging is by-design. It looks to me like your Service is keeping a reference to the HttpClient and that will keep the handler rooted. The service is registered as a singleton which I think means the service will be created once and kept alive for the lifetime of the host and the host will be kept alive by the cmdlet instance. The logging is just because you've enabled debug level logging and this debug statement is telling you the handler is still rooted by something after time has passed. /cc @davidfowl @maryamariyan to check my understanding here. FYI @dotnet/ncl |
That's right. |
Thanks people for comments. So I guess the Cmdlet class instance and |
I'm not too familiar on Cmdlet lifecycles but that's what it would seem like from what you shared. @daxian-dbw or @SteveL-MSFT might know. If you found that the lifetime of the Cmdlet was too long, you could always use a WeakReference for the host or the HttpClient and then recreate it as needed if it was no longer alive. |
You need to dispose the host in order to dispose the service provider which will dispose the other services. |
Yes, that's exactly what PowerShell does to invoke a cmdlet. If your cmdlet class implements Here is an example: using System;
using System.Management.Automation;
namespace dotnet3
{
[Cmdlet(VerbsCommon.New, "Test")]
public class NewTest : Cmdlet, IDisposable
{
protected override void BeginProcessing()
{
WriteVerbose("Processing");
}
public void Dispose()
{
Console.WriteLine("instance was disposed.");
}
}
} Invocation of |
Hi there, Thanks for all this info and sorry to be slow getting back to this due to overload. I was going to close this call except I still have a problem. I implemented IDisposable and added the following method to public void Dispose()
{
_host.StopAsync();
Console.WriteLine("Host stopped.");
} But this did not solve the issue even though logging shows the There is also an architecture decision here for me which someone might have a view on. Should I have a single IHost which is shared by all cmdlets and lives forever or should I spin up an IHost per cmdlet invocation making sure IHost setup/teardown works properly? Obviously the current situation of a new IHost every cmdlet invocation which is not disposed is undesirable but thinking what might be best to replace it with. (I will also update the I had some issues with one IHost per Xunit test recently and resolved these successfully by sharing an IHost between multiple Xunit tests via a fixture so maybe I should try a similar approach here to avoid IHost setup/teardown penalty. BR, |
Yes, you need to call IHost.Dispose |
Like this? public async void Dispose()
{
await _host.StopAsync();
_host.Dispose();
Console.WriteLine("Host disposed.");
} |
Unfortunately above code doesn't solve issue either.... |
No, not like like, like this: public async Dispose()
{
_host.Dispose();
Console.WriteLine("Host disposed.");
} |
Thanks @davidfowl . Sadly below which I think you meant still produces the problem.... public void Dispose()
{
_host.Dispose();
Console.WriteLine("Host disposed3.");
} |
Is it being called? |
Yeah, I put "3" on the log to make sure the code is called. PowerShell log....
|
Here's my test script. I tend to re-publish and import module into PowerShell when testing (as script shows) so I can re-build module in C# without having to make sure the DLL is unloaded from PowerShell. I simply restart PowerShell in VS Code and run to pick up latest module build. $VerbosePreference = "Continue"
[bool] $UsePublish = $true
if ($UsePublish) {
if (-not (Test-Path env:ModulePublished)) {
Push-Location C:\ReposTmp\PSModuleIssueRepro\PSModuleIssueRepro
dotnet publish
Pop-Location
$env:ModulePublished = 'true'
}
$importDir = "C:\ReposTmp\PSModuleIssueRepro\PSModuleIssueRepro\bin\Debug\netstandard2.1\publish\PSModuleIssueRepro.dll"
}
else {
$importDir = "C:\ReposTmp\PSModuleIssueRepro\PSModuleIssueRepro\bin\Debug\netstandard2.1\PSModuleIssueRepro.dll"
}
import-module $importDir
write-verbose ("Environment ready" + [Environment]::NewLine)
$myOutput = New-Test
write-verbose ("New-Test: output" + [Environment]::NewLine + ($myOutput | out-string).trim() + [Environment]::NewLine)
|
I have this same concern as well. except for me the remaining count keeps increasing.
so far i haven't seen the remaining item count decrease. it's counting over 15 items now. been running for a good several minutes now. wondering why the cleanup cycle doesn't seem to be doing anything I was thinking the httpClientFactory should be managing this. Should I expect that remaining count to drop back to 0? Basically I register a named http client with some retry policy extension method that applies a polly policy
then register a service that depends on IHttpClientFactory, which will use the factory to create the named client to issue a get request
then I have a hosted background service that depends on the Api data provider service and just keeps polling an api for new data on some wait interval
I also tried registering a typed httpClient and injecting an HttpClient dependency into MyApiDataProvider. If I just inject an httpClient into the typed client, and inject the typed client into the background service, the remaining count stays at 1 which makes since since the background service is holding onto that one instance. |
missing a using statement for the httpClient obtained from the factory. could that be my oversight. i think if the count stays at 1 that means previous instances are getting cleaned and perhaps the last used one is still holding but that should be OK. i set a breakpoint startup where the base address and authentication header gets set and the breakpoint gets hit each time which is expected. |
@tmp1k Did you find a solution? I also have a scoped service (AddScoped) with an injected http client (AddHttpClient). Disposing of the http client manually (Dispose()) doesn't seem to solve it. |
net 6, AddHttpClient() and named client configuration at composition root, using mostly transient services with injected IHttpClientFactory which creates named clients. Is using HttpClient more than that? I see plenty of cleanup in logs and my service that uses http client is not operational because of that :/ |
Ok, i've done my infinitive loop like this https://learn.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-5.0&tabs=visual-studio and now the server can be stopped.👌 |
Still no solution on this? All my services are transient but everytime HttpMessageHandler gets expired after 2 minutes - it justs stacks items for cleanup cycle and nothing is processed but sometimes after stacking some more items its just randomly processes some of (but not all) remaining items and overall unprocessed items keeps increasing And after long time of spamming in logs about cleanups it just sometimes cleanup them all. Is it intentional behaviour and I just have to supress these logs? |
This works as expected. The cleanup can only process the handlers that were garbage collected. Until your app triggers a GC, the clean up is not able to verify that the handlers are not used anymore. For the illustration purposes, here is an example of manually triggering Full GC (full src in the end of the post): services.AddKeyedTransient<DisposeTrackingHandler>(KeyedService.AnyKey);
services.ConfigureHttpClientDefaults(b => b.SetHandlerLifetime(TimeSpan.FromSeconds(5)));
services.AddHttpClient("discard")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("discard"));
// ...
_ = httpClientFactory.CreateClient("discard");
PrintAliveHandlers();
await Delay30s();
GcCollect();
await Delay30s();
PrintAliveHandlers();
// -----
void GcCollect()
{
log.LogInformation("GcCollect");
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
} Which shows that after GC the handler gets successfully processed and disposed:
If GC doesn't help, it is possible a handler is erroneously rooted (saved in a field or in a variable) for the prolonged period of time, so it can't be collected in time. services.AddHttpClient("rooted")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("rooted"));
services.AddHttpClient("weak-ref")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("weak-ref"));
// ...
log.LogInformation($"Creating 'rooted'");
var rootedClient = httpClientFactory.CreateClient("rooted");
log.LogInformation($"Creating 'weak-ref'");
var weakRefClient = httpClientFactory.CreateClient("weak-ref");
var weakRef = new WeakReference(weakRefClient);
weakRefClient = null;
PrintAliveHandlers();
await Delay30s();
GcCollect();
await Delay30s();
PrintAliveHandlers();
log.LogInformation("Unrooting 'rooted'");
rootedClient.Dispose();
rootedClient = null;
GcCollect();
await Delay30s();
PrintAliveHandlers(); You can see that
Complete runnable exampleusing System.Runtime.CompilerServices;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
var services = new ServiceCollection();
services.AddLogging(b => b
.ClearProviders()
.AddConsole()
.SetMinimumLevel(LogLevel.Trace));
services.AddKeyedTransient<DisposeTrackingHandler>(KeyedService.AnyKey);
services.ConfigureHttpClientDefaults(b => b.SetHandlerLifetime(TimeSpan.FromSeconds(5)));
services.AddHttpClient("rooted")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("rooted"));
services.AddHttpClient("weak-ref")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("weak-ref"));
services.AddHttpClient("discard")
.AddHttpMessageHandler(sp => sp.GetRequiredKeyedService<DisposeTrackingHandler>("discard"));
var serviceProvider = services.BuildServiceProvider();
// ---
var log = serviceProvider.GetRequiredService<ILogger<Program>>();
var httpClientFactory = serviceProvider.GetRequiredService<IHttpClientFactory>();
log.LogInformation($"Creating 'rooted'");
var rootedClient = httpClientFactory.CreateClient("rooted");
log.LogInformation($"Creating 'weak-ref'");
var weakRefClient = httpClientFactory.CreateClient("weak-ref");
var weakRef = new WeakReference(weakRefClient);
weakRefClient = null;
log.LogInformation($"Creating 'discard'");
_ = httpClientFactory.CreateClient("discard");
// ---
PrintAliveHandlers();
await Delay30s();
GcCollect();
await Delay30s();
PrintAliveHandlers();
log.LogInformation("Unrooting 'rooted'");
rootedClient.Dispose();
rootedClient = null;
GcCollect();
await Delay30s();
PrintAliveHandlers();
// ----------------------------------------------------------------
void GcCollect()
{
log.LogInformation("GcCollect");
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
}
async Task Delay30s()
{
log.LogTrace("Delay30s");
await Task.Delay(TimeSpan.FromSeconds(30));
}
void PrintAliveHandlers()
{
log.LogInformation("Alive handlers: [ {list} ]", DisposeTrackingHandler.GetNames());
}
// ----------------------------------------------------------------
class DisposeTrackingHandler : DelegatingHandler
{
private static readonly List<string> _instanceNames = [];
public static string GetNames()
{
lock (_instanceNames)
{
return string.Join(", ", _instanceNames);
}
}
private readonly ILogger<DisposeTrackingHandler> _log;
private readonly string _name;
private bool _disposed;
public DisposeTrackingHandler([ServiceKey] string name, ILogger<DisposeTrackingHandler> log)
{
_log = log;
_name = name;
Trace();
lock (_instanceNames)
{
_instanceNames.Add(_name);
}
}
protected override void Dispose(bool disposing)
{
if (_disposed)
{
return;
}
_disposed = true;
Trace();
base.Dispose(disposing);
lock (_instanceNames)
{
_instanceNames.Remove(_name);
}
}
private void Trace([CallerMemberName] string? memberName = null) => _log.LogTrace("_name='{name}', {memberName}", _name, memberName ?? "(?)");
} |
Triage: Closing as answered above. |
area-Extensions-HttpClientFactory
Hi there,
I have a c# PowerShell module with cmdlets that make use of the .NET generic host. It uses .NET standard 2.1.
Unfortunately after a few calls to the cmdlets I start to get endless debug messages in the PowerShell terminal....
I looked at this thread (aspnet/HttpClientFactory#165) and used JetBrains' dotMemory to force a garbage collection but it doesn't fix it.
I did the shortest path analysis below but to be honest I'm not sure entirely how to interpret it.
Any help appreciated....
The text was updated successfully, but these errors were encountered: