Merge pull request #50 from Lombiq/issue/NEST-361-dev-repro

NEST-361: Fix memory leak in idle tenant shutdown
Lombiq · Jul 3, 2023 · 56a6eed · 56a6eed
2 parents 088a8ba + 624bf37
commit 56a6eed
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 43 deletions.
diff --git a/Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI/Constants/IdleTenantData.cs b/Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI/Constants/IdleTenantData.cs
@@ -0,0 +1,8 @@
+namespace Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI.Constants;
+
+public static class IdleTenantData
+{
+    public const string IdleTenantName = nameof(IdleTenantName);
+    public const string IdleTenantPrefix = "idletenantprefix";
+    public const string IdleTenantRecipe = "Lombiq.OSOCE.Tests";
+}
diff --git a/...osting.Tenants.IdleTenantManagement.Tests.UI/Extensions/IdleTenantManagementExtensions.cs b/...osting.Tenants.IdleTenantManagement.Tests.UI/Extensions/IdleTenantManagementExtensions.cs
@@ -3,6 +3,7 @@
 using Shouldly;
 using System;
 using System.Threading.Tasks;
+using static Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI.Constants.IdleTenantData;
 
 namespace Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI.Extensions;
 
@@ -26,5 +27,5 @@ public static void SetMaxIdleMinutesAndLoggingForUITest(
     public static readonly Func<IWebApplicationInstance, Task> AssertAppLogsWithIdleCheckAsync =
         async webApplicationInstance =>
             (await webApplicationInstance.GetLogOutputAsync())
-            .ShouldContain("Shutting down tenant \"Default\" because of idle timeout.");
+            .ShouldContain($"Shutting down tenant \"{IdleTenantName}\" because of idle timeout.");
 }
diff --git a/...sting.Tenants.IdleTenantManagement.Tests.UI/Extensions/TestCaseUITestContextExtensions.cs b/...sting.Tenants.IdleTenantManagement.Tests.UI/Extensions/TestCaseUITestContextExtensions.cs
@@ -1,13 +1,32 @@
 using Lombiq.Tests.UI.Extensions;
+using Lombiq.Tests.UI.Pages;
 using Lombiq.Tests.UI.Services;
 using System.Threading.Tasks;
+using static Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI.Constants.IdleTenantData;
 
 namespace Lombiq.Hosting.Tenants.IdleTenantManagement.Tests.UI.Extensions;
 
 public static class TestCaseUITestContextExtensions
 {
-    public static async Task TestIdleTenantManagerBehaviorAsync(this UITestContext context)
+    public static async Task TestIdleTenantManagerBehaviorAsync(
+        this UITestContext context)
     {
+        // Setting up new tenant to test the feature
+        await context.CreateAndSwitchToTenantManuallyAsync(IdleTenantName, IdleTenantPrefix, string.Empty);
+
+        // Because this test is aimed at a single tenant's behavior we don't need dynamic tenant data.
+        // The used constants here can be found at IdleTenantManagement.Tests.UI/Constants/IdleTenantData.
+        await context.GoToSetupPageAndSetupOrchardCoreAsync(
+            new OrchardCoreSetupParameters(context)
+            {
+                SiteName = IdleTenantName,
+                RecipeId = IdleTenantRecipe,
+                TablePrefix = IdleTenantName,
+                RunSetupOnCurrentPage = true,
+            });
+
+        await context.SignInDirectlyAsync();
+
         // We are letting the site to sit idle for more than two minutes so that the tenant could be shut down by the
         // background task.
         await Task.Delay(129420);

diff --git a/Lombiq.Hosting.Tenants.IdleTenantManagement/Services/IdleShutdownTask.cs b/Lombiq.Hosting.Tenants.IdleTenantManagement/Services/IdleShutdownTask.cs
@@ -16,54 +16,25 @@ public class IdleShutdownTask : IBackgroundTask
 {
     public async Task DoWorkAsync(IServiceProvider serviceProvider, CancellationToken cancellationToken)
     {
-        var clock = serviceProvider.GetService<IClock>();
-        var lastActiveTimeAccessor = serviceProvider.GetService<ILastActiveTimeAccessor>();
-        var shellHost = serviceProvider.GetService<IShellHost>();
+        var maxIdleMinutes = serviceProvider.GetRequiredService<IOptions<IdleShutdownOptions>>().Value.MaxIdleMinutes;
 
-        var options = serviceProvider.GetService<IOptions<IdleShutdownOptions>>();
-        var maxIdleMinutes = options.Value.MaxIdleMinutes;
+        var shellSettings = serviceProvider.GetRequiredService<ShellSettings>();
 
-        if (maxIdleMinutes <= 0) return;
+        if (maxIdleMinutes <= 0 || shellSettings.IsDefaultShell()) return;
 
-        if (lastActiveTimeAccessor.LastActiveDateTimeUtc.AddMinutes(maxIdleMinutes) <= clock?.UtcNow)
-        {
-            var shellSettings = serviceProvider.GetService<ShellSettings>();
-            var logger = serviceProvider.GetService<ILogger<IdleShutdownTask>>();
+        var clock = serviceProvider.GetRequiredService<IClock>();
 
-            logger?.LogInformation("Shutting down tenant \"{ShellName}\" because of idle timeout.", shellSettings?.Name);
+        var lastActiveDateTimeUtc = serviceProvider.GetRequiredService<ILastActiveTimeAccessor>().LastActiveDateTimeUtc;
 
-            try
-            {
-                await InvokeShutdownAsync(shellSettings, shellHost);
-            }
-            catch (Exception e)
-            {
-                logger?.LogError(
-                    e,
-                    "Shutting down \"{ShellName}\" because of idle timeout failed with the following exception. Another shutdown will be attempted.",
-                    shellSettings?.Name);
+        if (lastActiveDateTimeUtc.AddMinutes(maxIdleMinutes) <= clock?.UtcNow)
+        {
+            var logger = serviceProvider.GetRequiredService<ILogger<IdleShutdownTask>>();
 
-                // If the ReleaseShellContextAsync() fails (which can happen with a DB error: then the transaction
-                // commits triggered by the dispose will fail) then while the tenant is unavailable the shell is still
-                // active in a failed state. So first we need to correctly start the tenant, then shut it down for good.
+            logger?.LogWarning("Shutting down tenant \"{ShellName}\" because of idle timeout.", shellSettings.Name);
 
-                var shellSettingsManager = serviceProvider.GetService<IShellSettingsManager>();
+            var shellHost = serviceProvider.GetRequiredService<IShellHost>();
 
-                await InvokeRestartAsync(shellSettingsManager, shellHost, shellSettings);
-                await InvokeShutdownAsync(shellSettings, shellHost);
-            }
+            await shellHost.ReleaseShellContextAsync(shellSettings, eventSource: false);
         }
     }
-
-    private static Task InvokeShutdownAsync(ShellSettings shellSettings, IShellHost shellHost) =>
-        shellHost.ReleaseShellContextAsync(shellSettings);
-
-    private static async Task InvokeRestartAsync(
-        IShellSettingsManager shellSettingsManager,
-        IShellHost shellHost,
-        ShellSettings shellSettings)
-    {
-        await shellSettingsManager.SaveSettingsAsync(shellSettings);
-        await shellHost.UpdateShellSettingsAsync(shellSettings);
-    }
 }
diff --git a/Lombiq.Hosting.Tenants.IdleTenantManagement/Startup.cs b/Lombiq.Hosting.Tenants.IdleTenantManagement/Startup.cs
@@ -25,7 +25,7 @@ public override void Configure(
         IApplicationBuilder app,
         IEndpointRouteBuilder routes,
         IServiceProvider serviceProvider) =>
-        app.UseMiddleware<IdleTimeProviderMiddleware>();
+            app.UseMiddleware<IdleTimeProviderMiddleware>();
 
     public override void ConfigureServices(IServiceCollection services)
     {