Ensure TypeModuleChangeMonitor is properly disposed (#8041)

Author: tobias-tengler

src/HotChocolate/Core/src/Execution/RequestExecutorResolver.cs

@@ -96,16 +96,28 @@ await _optionsMonitor.GetAsync(schemaName, cancellationToken)
                 setup.SchemaBuilder ?? new SchemaBuilder(),
                 _applicationServices);
 
+
+            var typeModuleChangeMonitor = new TypeModuleChangeMonitor(this, context.SchemaName);
+
+            // if there are any type modules we will register them with the
+            // type module change monitor.
+            // The module will track if type modules signal changes to the schema and
+            // start a schema eviction.
+            foreach (var typeModule in setup.TypeModules)
+            {
+                typeModuleChangeMonitor.Register(typeModule);
+            }
+
             var schemaServices =
-                await CreateSchemaServicesAsync(context, setup, cancellationToken)
+                await CreateSchemaServicesAsync(context, setup, typeModuleChangeMonitor, cancellationToken)
                     .ConfigureAwait(false);
 
             registeredExecutor = new RegisteredExecutor(
                 schemaServices.GetRequiredService<IRequestExecutor>(),
                 schemaServices,
                 schemaServices.GetRequiredService<IExecutionDiagnosticEvents>(),
                 setup,
-                schemaServices.GetRequiredService<TypeModuleChangeMonitor>());
+                typeModuleChangeMonitor);
 
             var executor = registeredExecutor.Executor;
 
@@ -131,24 +143,26 @@ public void EvictRequestExecutor(string? schemaName = default)
     {
         schemaName ??= Schema.DefaultName;
 
-        if (_executors.TryRemove(schemaName, out var re))
+        if (_executors.TryRemove(schemaName, out var executor))
         {
-            re.DiagnosticEvents.ExecutorEvicted(schemaName, re.Executor);
+            executor.DiagnosticEvents.ExecutorEvicted(schemaName, executor.Executor);
 
             try
             {
+                executor.TypeModuleChangeMonitor.Dispose();
+
                 RequestExecutorEvicted?.Invoke(
                     this,
-                    new RequestExecutorEvictedEventArgs(schemaName, re.Executor));
+                    new RequestExecutorEvictedEventArgs(schemaName, executor.Executor));
                 _events.RaiseEvent(
                     new RequestExecutorEvent(
                         RequestExecutorEventType.Evicted,
                         schemaName,
-                        re.Executor));
+                        executor.Executor));
             }
             finally
             {
-                BeginRunEvictionEvents(re);
+                BeginRunEvictionEvents(executor);
             }
         }
     }
@@ -157,26 +171,7 @@ private void EvictAllRequestExecutors()
     {
         foreach (var key in _executors.Keys)
         {
-            if (_executors.TryRemove(key, out var re))
-            {
-                re.DiagnosticEvents.ExecutorEvicted(key, re.Executor);
-
-                try
-                {
-                    RequestExecutorEvicted?.Invoke(
-                        this,
-                        new RequestExecutorEvictedEventArgs(key, re.Executor));
-                    _events.RaiseEvent(
-                        new RequestExecutorEvent(
-                            RequestExecutorEventType.Evicted,
-                            key,
-                            re.Executor));
-                }
-                finally
-                {
-                    BeginRunEvictionEvents(re);
-                }
-            }
+            EvictRequestExecutor(key);
         }
     }
 
@@ -201,6 +196,7 @@ private static async Task RunEvictionEvents(RegisteredExecutor registeredExecuto
     private async Task<IServiceProvider> CreateSchemaServicesAsync(
         ConfigurationContext context,
         RequestExecutorSetup setup,
+        TypeModuleChangeMonitor typeModuleChangeMonitor,
         CancellationToken cancellationToken)
     {
         ulong version;
@@ -211,22 +207,12 @@ private async Task<IServiceProvider> CreateSchemaServicesAsync(
         }
 
         var serviceCollection = new ServiceCollection();
-        var typeModuleChangeMonitor = new TypeModuleChangeMonitor(this, context.SchemaName);
         var lazy = new SchemaBuilder.LazySchema();
 
         var executorOptions =
             await OnConfigureRequestExecutorOptionsAsync(context, setup, cancellationToken)
                 .ConfigureAwait(false);
 
-        // if there are any type modules we will register them with the
-        // type module change monitor.
-        // The module will track if type modules signal changes to the schema and
-        // start a schema eviction.
-        foreach (var typeModule in setup.TypeModules)
-        {
-            typeModuleChangeMonitor.Register(typeModule);
-        }
-
         // we allow newer type modules to apply configurations.
         await typeModuleChangeMonitor.ConfigureAsync(context, cancellationToken)
             .ConfigureAwait(false);
@@ -241,7 +227,6 @@ await typeModuleChangeMonitor.ConfigureAsync(context, cancellationToken)
                 setup.DefaultPipelineFactory,
                 setup.Pipeline));
 
-        serviceCollection.AddSingleton(typeModuleChangeMonitor);
         serviceCollection.AddSingleton(executorOptions);
         serviceCollection.AddSingleton<IRequestExecutorOptionsAccessor>(
             static s => s.GetRequiredService<RequestExecutorOptions>());

src/HotChocolate/Core/test/Execution.Tests/Configuration/TypeModuleTests.cs

@@ -3,6 +3,7 @@
 using HotChocolate.Types.Descriptors;
 using HotChocolate.Types.Descriptors.Definitions;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Hosting;
 
 namespace HotChocolate.Execution.Configuration;
 
@@ -64,6 +65,58 @@ public async Task Use_Type_Module_From_Factory()
             .MatchSnapshotAsync();
     }
 
+    [Fact]
+    public async Task Ensure_Warmups_Are_Triggered_An_Appropriate_Number_Of_Times()
+    {
+        // arrange
+        var typeModule = new TriggerableTypeModule();
+        var warmups = 0;
+        var resetEvent = new AutoResetEvent(false);
+
+        var services = new ServiceCollection();
+        services
+            .AddGraphQL()
+            .AddTypeModule(_ => typeModule)
+            .InitializeOnStartup(keepWarm: true, warmup: (_, _) =>
+            {
+                warmups++;
+                resetEvent.Set();
+                return Task.CompletedTask;
+            })
+            .AddQueryType(d => d.Field("foo").Resolve(""));
+        var provider = services.BuildServiceProvider();
+        var warmupService = provider.GetRequiredService<IHostedService>();
+
+        using var cts = new CancellationTokenSource();
+        _ = Task.Run(async () =>
+        {
+            await warmupService.StartAsync(CancellationToken.None);
+        }, cts.Token);
+
+        var resolver = provider.GetRequiredService<IRequestExecutorResolver>();
+
+        await resolver.GetRequestExecutorAsync(null, cts.Token);
+
+        // act
+        // assert
+        typeModule.TriggerChange();
+        resetEvent.WaitOne();
+
+        // 2 since we have the initial warmup at "startup" and the one triggered above.
+        Assert.Equal(2, warmups);
+
+        resetEvent.Reset();
+        typeModule.TriggerChange();
+        resetEvent.WaitOne();
+
+        Assert.Equal(3, warmups);
+    }
+
+    private sealed class TriggerableTypeModule : TypeModule
+    {
+        public void TriggerChange() => OnTypesChanged();
+    }
+
     public class DummyTypeModule : ITypeModule
     {
 #pragma warning disable CS0067