diff --git a/src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Extensions/ServiceCollectionExtensions.cs b/src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Extensions/ServiceCollectionExtensions.cs index e20b0deb656..d09bac6fd65 100644 --- a/src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/HotChocolate/Adapters/src/Adapters.Mcp.Core/Extensions/ServiceCollectionExtensions.cs @@ -58,6 +58,17 @@ public static void AddMcpSchemaServices( var mcpManager = applicationServices.GetRequiredService(); var setup = mcpManager.GetSetup(schemaName); + // Root services are registered as instances so that the schema service provider never disposes them. + if (applicationServices.GetService() is { } loggerFactory) + { + services.AddSingleton(loggerFactory); + } + + if (applicationServices.GetService() is { } applicationLifetime) + { + services.AddSingleton(applicationLifetime); + } + services.AddLogging(); services.TryAddSingleton( @@ -99,16 +110,7 @@ public static void AddMcpSchemaServices( mcpManager.Get(schemaName).Storage, sp.GetRequiredService())); - services - .AddSingleton( - static sp => sp - .GetRequiredService().ServiceProvider - .GetRequiredService()) - .AddSingleton( - static sp => sp - .GetRequiredService().ServiceProvider - .GetRequiredService()) - .AddSingleton(); + services.AddSingleton(); var mcpServers = new ConcurrentDictionary(); services.AddSingleton(mcpServers); diff --git a/src/HotChocolate/Adapters/test/Adapters.Mcp.Tests/IntegrationTestBase.cs b/src/HotChocolate/Adapters/test/Adapters.Mcp.Tests/IntegrationTestBase.cs index 3bbc5ddb598..cd977f6e39f 100644 --- a/src/HotChocolate/Adapters/test/Adapters.Mcp.Tests/IntegrationTestBase.cs +++ b/src/HotChocolate/Adapters/test/Adapters.Mcp.Tests/IntegrationTestBase.cs @@ -12,6 +12,7 @@ using HotChocolate.Types; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.IdentityModel.Tokens; using ModelContextProtocol; @@ -1601,6 +1602,24 @@ await CreateTestServerAsync( Assert.Equal(TimeSpan.FromSeconds(10), options.InitializationTimeout); } + [Fact] + public async Task AddMcp_Should_NotDisposeRootLoggerFactory_When_SchemaServicesAreDisposed() + { + // arrange + var server = await CreateTestServerAsync(new TestMcpStorage()); + var loggerFactory = server.Services.GetRequiredService(); + var executor = await server.Services.GetRequiredService().GetExecutorAsync( + cancellationToken: TestContext.Current.CancellationToken); + var schemaLoggerFactory = executor.Schema.Services.GetRequiredService(); + + // act + await ((IAsyncDisposable)executor.Schema.Services).DisposeAsync(); + + // assert + Assert.Same(loggerFactory, schemaLoggerFactory); + Assert.Null(Record.Exception(() => loggerFactory.CreateLogger("Test"))); + } + protected abstract Task CreateTestServerAsync( IMcpStorage storage, ITypeDefinition[]? additionalTypes = null, diff --git a/src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorBuilderExtensions.Caches.cs b/src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorBuilderExtensions.Caches.cs index 02d933e6c0f..3f11dc6cc64 100644 --- a/src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorBuilderExtensions.Caches.cs +++ b/src/HotChocolate/Core/src/Types/Execution/DependencyInjection/RequestExecutorBuilderExtensions.Caches.cs @@ -1,4 +1,3 @@ -using HotChocolate; using HotChocolate.Execution.Caching; using HotChocolate.Execution.Configuration; using HotChocolate.Language; @@ -26,12 +25,11 @@ internal static IRequestExecutorBuilder AddDocumentCache(this IRequestExecutorBu return new DefaultDocumentCache(options.OperationDocumentCacheSize); }); + var schemaName = builder.Name; + + // The document cache is registered as an instance so that the schema service provider never disposes it. return builder.ConfigureSchemaServices( - static (applicationServices, s) => - s.AddSingleton(schemaServices => - { - var schemaName = schemaServices.GetRequiredService().Name; - return applicationServices.GetRequiredKeyedService(schemaName); - })); + (applicationServices, s) => + s.AddSingleton(applicationServices.GetRequiredKeyedService(schemaName))); } } diff --git a/src/HotChocolate/Core/test/Execution.Tests/DocumentCacheTests.cs b/src/HotChocolate/Core/test/Execution.Tests/DocumentCacheTests.cs index b213f624ac5..4eb80903201 100644 --- a/src/HotChocolate/Core/test/Execution.Tests/DocumentCacheTests.cs +++ b/src/HotChocolate/Core/test/Execution.Tests/DocumentCacheTests.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using HotChocolate.Language; using HotChocolate.Types; using Microsoft.Extensions.DependencyInjection; @@ -95,4 +96,47 @@ public async Task Document_Cache_Should_Be_Scoped_To_Schema() Assert.NotSame(executorB, executorA); Assert.NotSame(documentCacheB, documentCacheA); } + + [Fact] + public async Task AddGraphQL_Should_NotDisposeDocumentCache_When_SchemaServicesAreDisposed() + { + // arrange + var documentCache = new DisposableDocumentCache(); + var services = new ServiceCollection(); + services.AddKeyedSingleton(ISchemaDefinition.DefaultName, documentCache); + services + .AddGraphQL() + .AddQueryType(d => d.Field("foo").Resolve("")); + var executor = await services.BuildServiceProvider().GetRequestExecutorAsync( + cancellationToken: TestContext.Current.CancellationToken); + var schemaDocumentCache = executor.Schema.Services.GetRequiredService(); + + // act + await ((IAsyncDisposable)executor.Schema.Services).DisposeAsync(); + + // assert + Assert.Same(documentCache, schemaDocumentCache); + Assert.False(documentCache.IsDisposed); + } + + private sealed class DisposableDocumentCache : IDocumentCache, IDisposable + { + public bool IsDisposed { get; private set; } + + public int Capacity => 0; + + public int Count => 0; + + public bool TryGetDocument(string documentId, [NotNullWhen(true)] out CachedDocument? document) + { + document = null; + return false; + } + + public void TryAddDocument(string documentId, CachedDocument document) + { + } + + public void Dispose() => IsDisposed = true; + } } diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/CoreFusionGatewayBuilderExtensions.ErrorFilter.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/CoreFusionGatewayBuilderExtensions.ErrorFilter.cs index 6f07960da8a..defff2cdd32 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/CoreFusionGatewayBuilderExtensions.ErrorFilter.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/CoreFusionGatewayBuilderExtensions.ErrorFilter.cs @@ -56,8 +56,10 @@ public static IFusionGatewayBuilder AddErrorFilter( ArgumentNullException.ThrowIfNull(builder); builder.Services.TryAddSingleton(); + + // The error filter is registered as an instance so that the schema service provider never disposes it. return builder.ConfigureSchemaServices( - (sp, s) => s.AddSingleton(_ => sp.GetRequiredService())); + (sp, s) => s.AddSingleton(sp.GetRequiredService())); } /// diff --git a/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/HotChocolateFusionServiceCollectionExtensions.cs b/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/HotChocolateFusionServiceCollectionExtensions.cs index 4dcc81c2234..2134ef52a7b 100644 --- a/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/HotChocolateFusionServiceCollectionExtensions.cs +++ b/src/HotChocolate/Fusion/src/Fusion.Execution/DependencyInjection/HotChocolateFusionServiceCollectionExtensions.cs @@ -133,12 +133,11 @@ private static IFusionGatewayBuilder AddDocumentCache(this IFusionGatewayBuilder return new DefaultDocumentCache(options.OperationDocumentCacheSize); }); + var schemaName = builder.Name; + + // The document cache is registered as an instance so that the schema service provider never disposes it. return builder.ConfigureSchemaServices( - static (applicationServices, s) => - s.AddSingleton(schemaServices => - { - var schemaName = schemaServices.GetRequiredService().Name; - return applicationServices.GetRequiredKeyedService(schemaName); - })); + (applicationServices, s) => + s.AddSingleton(applicationServices.GetRequiredKeyedService(schemaName))); } } diff --git a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/ErrorFilterTests.cs b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/ErrorFilterTests.cs index 724ffbdf7f5..3f7a727b906 100644 --- a/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/ErrorFilterTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/ErrorFilterTests.cs @@ -320,6 +320,38 @@ type Query { """); } + [Fact] + public async Task AddErrorFilter_Should_NotDisposeErrorFilter_When_EvictedExecutorIsDisposed() + { + // arrange + using var server1 = CreateSourceSchema( + "A", + """ + type Query { + field: String + } + """ + ); + + using var gateway = await CreateCompositeSchemaAsync( + [ + ("A", server1) + ], + configureGatewayBuilder: b => b.AddErrorFilter()); + var errorFilter = gateway.Services.GetRequiredService(); + var executor = await gateway.Services.GetRequestExecutorAsync( + cancellationToken: TestContext.Current.CancellationToken); + var schemaErrorFilter = Assert.Single( + executor.Schema.Services.GetServices().OfType()); + + // act + await ((IAsyncDisposable)executor).DisposeAsync(); + + // assert + Assert.Same(errorFilter, schemaErrorFilter); + Assert.False(errorFilter.IsDisposed); + } + public class DummyErrorFilter : IErrorFilter { public IError OnError(IError error) @@ -339,4 +371,13 @@ public IError OnError(IError error) #pragma warning restore CS9113 // Parameter is unread. public class SomeService; + + private sealed class DisposableErrorFilter : IErrorFilter, IDisposable + { + public bool IsDisposed { get; private set; } + + public IError OnError(IError error) => error; + + public void Dispose() => IsDisposed = true; + } } diff --git a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Execution/FusionDocumentCacheTests.cs b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Execution/FusionDocumentCacheTests.cs index 193cc9c8986..be1ea93c66b 100644 --- a/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Execution/FusionDocumentCacheTests.cs +++ b/src/HotChocolate/Fusion/test/Fusion.Execution.Tests/Execution/FusionDocumentCacheTests.cs @@ -1,3 +1,4 @@ +using System.Diagnostics.CodeAnalysis; using HotChocolate.Execution; using HotChocolate.Language; using Microsoft.Extensions.DependencyInjection; @@ -128,4 +129,53 @@ type Query { Assert.NotSame(executorB, executorA); Assert.NotSame(documentCacheB, documentCacheA); } + + [Fact] + public async Task AddGraphQLGateway_Should_NotDisposeDocumentCache_When_EvictedExecutorIsDisposed() + { + // arrange + var documentCache = new DisposableDocumentCache(); + var services = new ServiceCollection(); + services.AddKeyedSingleton(ISchemaDefinition.DefaultName, documentCache); + services + .AddGraphQLGateway() + .AddInMemoryConfiguration( + ComposeSchemaDocument( + """ + type Query { + field: String! + } + """)); + var executor = await services.BuildServiceProvider().GetRequestExecutorAsync( + cancellationToken: TestContext.Current.CancellationToken); + var schemaDocumentCache = executor.Schema.Services.GetRequiredService(); + + // act + await ((IAsyncDisposable)executor).DisposeAsync(); + + // assert + Assert.Same(documentCache, schemaDocumentCache); + Assert.False(documentCache.IsDisposed); + } + + private sealed class DisposableDocumentCache : IDocumentCache, IDisposable + { + public bool IsDisposed { get; private set; } + + public int Capacity => 0; + + public int Count => 0; + + public bool TryGetDocument(string documentId, [NotNullWhen(true)] out CachedDocument? document) + { + document = null; + return false; + } + + public void TryAddDocument(string documentId, CachedDocument document) + { + } + + public void Dispose() => IsDisposed = true; + } }