Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ public static void AddMcpSchemaServices(
var mcpManager = applicationServices.GetRequiredService<McpManager>();
var setup = mcpManager.GetSetup(schemaName);

// Root services are registered as instances so that the schema service provider never disposes them.
if (applicationServices.GetService<ILoggerFactory>() is { } loggerFactory)
{
services.AddSingleton(loggerFactory);
}

if (applicationServices.GetService<IHostApplicationLifetime>() is { } applicationLifetime)
{
services.AddSingleton(applicationLifetime);
}

services.AddLogging();

services.TryAddSingleton(
Expand Down Expand Up @@ -99,16 +110,7 @@ public static void AddMcpSchemaServices(
mcpManager.Get(schemaName).Storage,
sp.GetRequiredService<IMcpDiagnosticEvents>()));

services
.AddSingleton(
static sp => sp
.GetRequiredService<IRootServiceProviderAccessor>().ServiceProvider
.GetRequiredService<IHostApplicationLifetime>())
.AddSingleton(
static sp => sp
.GetRequiredService<IRootServiceProviderAccessor>().ServiceProvider
.GetRequiredService<ILoggerFactory>())
.AddSingleton<McpFeatureRegistry>();
services.AddSingleton<McpFeatureRegistry>();

var mcpServers = new ConcurrentDictionary<string, McpServer>();
services.AddSingleton(mcpServers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ILoggerFactory>();
var executor = await server.Services.GetRequiredService<IRequestExecutorProvider>().GetExecutorAsync(
cancellationToken: TestContext.Current.CancellationToken);
var schemaLoggerFactory = executor.Schema.Services.GetRequiredService<ILoggerFactory>();

// act
await ((IAsyncDisposable)executor.Schema.Services).DisposeAsync();

// assert
Assert.Same(loggerFactory, schemaLoggerFactory);
Assert.Null(Record.Exception(() => loggerFactory.CreateLogger("Test")));
}

protected abstract Task<TestServer> CreateTestServerAsync(
IMcpStorage storage,
ITypeDefinition[]? additionalTypes = null,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using HotChocolate;
using HotChocolate.Execution.Caching;
using HotChocolate.Execution.Configuration;
using HotChocolate.Language;
Expand Down Expand Up @@ -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<ISchemaDefinition>().Name;
return applicationServices.GetRequiredKeyedService<IDocumentCache>(schemaName);
}));
(applicationServices, s) =>
s.AddSingleton(applicationServices.GetRequiredKeyedService<IDocumentCache>(schemaName)));
}
}
44 changes: 44 additions & 0 deletions src/HotChocolate/Core/test/Execution.Tests/DocumentCacheTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using HotChocolate.Language;
using HotChocolate.Types;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -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<IDocumentCache>(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<IDocumentCache>();

// 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ public static IFusionGatewayBuilder AddErrorFilter(
ArgumentNullException.ThrowIfNull(builder);

builder.Services.TryAddSingleton<T>();

// The error filter is registered as an instance so that the schema service provider never disposes it.
return builder.ConfigureSchemaServices(
(sp, s) => s.AddSingleton<IErrorFilter, T>(_ => sp.GetRequiredService<T>()));
(sp, s) => s.AddSingleton<IErrorFilter>(sp.GetRequiredService<T>()));
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ISchemaDefinition>().Name;
return applicationServices.GetRequiredKeyedService<IDocumentCache>(schemaName);
}));
(applicationServices, s) =>
s.AddSingleton(applicationServices.GetRequiredKeyedService<IDocumentCache>(schemaName)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<DisposableErrorFilter>());
var errorFilter = gateway.Services.GetRequiredService<DisposableErrorFilter>();
var executor = await gateway.Services.GetRequestExecutorAsync(
cancellationToken: TestContext.Current.CancellationToken);
var schemaErrorFilter = Assert.Single(
executor.Schema.Services.GetServices<IErrorFilter>().OfType<DisposableErrorFilter>());

// act
await ((IAsyncDisposable)executor).DisposeAsync();

// assert
Assert.Same(errorFilter, schemaErrorFilter);
Assert.False(errorFilter.IsDisposed);
}

public class DummyErrorFilter : IErrorFilter
{
public IError OnError(IError error)
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using HotChocolate.Execution;
using HotChocolate.Language;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -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<IDocumentCache>(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<IDocumentCache>();

// 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;
}
}
Loading