Merge pull request #17791 from Shadowghost/fix-code-migration

Don't dispose application singletons after running a code migration

2 files changed, 166 insertions, 0 deletions

diff --git a/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs
new file mode 100644
index 0000000000..3bd8581a5f
--- /dev/null
+++ b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs
@@ -0,0 +1,112 @@
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Server.Migrations;
+using Jellyfin.Server.Migrations.Stages;
+using Jellyfin.Server.ServerSetupApp;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging.Abstractions;
+using Xunit;
+
+namespace Jellyfin.Server.Tests.Migrations;
+
+public class CodeMigrationTests
+{
+    [Fact]
+    public async Task Perform_LeavesApplicationSingletonsAlive()
+    {
+        var services = new ServiceCollection()
+            .AddLogging()
+            .RegisterStartupLogger()
+            .AddSingleton<ApplicationSingleton>()
+            .AddTransient<MigrationTransient>();
+
+        await using var serviceProvider = services.BuildServiceProvider();
+        var applicationSingleton = serviceProvider.GetRequiredService<ApplicationSingleton>();
+        var logger = new StartupLogger(NullLogger.Instance).BeginGroup($"Test migration");
+
+        var migration = new CodeMigration(
+            typeof(TestMigration),
+            new JellyfinMigrationAttribute("2026-09-05T10:00:00", nameof(TestMigration)),
+            null);
+        await migration.Perform(serviceProvider, logger, CancellationToken.None);
+
+        var performed = TestMigration.Performed;
+        Assert.NotNull(performed);
+        // The migration has to run against the applications own services, and they have to outlive it.
+        Assert.Same(applicationSingleton, performed.Singleton);
+        Assert.False(applicationSingleton.IsDisposed);
+        Assert.Same(applicationSingleton, serviceProvider.GetRequiredService<ApplicationSingleton>());
+        // Services created for the migration itself are still owned by the migration.
+        Assert.True(performed.Transient.IsDisposed);
+        // The startup logger has to stay attached to the topic of the running migration.
+        Assert.Same(logger.Topic, performed.Logger.Topic);
+    }
+
+    [Fact]
+    public async Task Perform_DoesNotLeakTheMigrationTopic()
+    {
+        var services = new ServiceCollection()
+            .AddLogging()
+            .RegisterStartupLogger()
+            .AddSingleton<ApplicationSingleton>()
+            .AddTransient<MigrationTransient>();
+
+        await using var serviceProvider = services.BuildServiceProvider();
+        var logger = new StartupLogger(NullLogger.Instance).BeginGroup($"Test migration");
+
+        var migration = new CodeMigration(
+            typeof(TestMigration),
+            new JellyfinMigrationAttribute("2026-09-05T10:00:00", nameof(TestMigration)),
+            null);
+        await migration.Perform(serviceProvider, logger, CancellationToken.None);
+
+        // The topic belongs to the migration that ran, so loggers resolved afterwards must not still write into it.
+        Assert.Null(serviceProvider.GetRequiredService<IStartupLogger<CodeMigrationTests>>().Topic);
+        Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
+    }
+
+    private sealed class ApplicationSingleton : IDisposable
+    {
+        public bool IsDisposed { get; private set; }
+
+        public void Dispose()
+        {
+            IsDisposed = true;
+        }
+    }
+
+    private sealed class MigrationTransient : IDisposable
+    {
+        public bool IsDisposed { get; private set; }
+
+        public void Dispose()
+        {
+            IsDisposed = true;
+        }
+    }
+
+    private sealed class TestMigration : IAsyncMigrationRoutine
+    {
+        public TestMigration(ApplicationSingleton singleton, MigrationTransient transient, IStartupLogger<TestMigration> logger)
+        {
+            Singleton = singleton;
+            Transient = transient;
+            Logger = logger;
+        }
+
+        public static TestMigration? Performed { get; private set; }
+
+        public ApplicationSingleton Singleton { get; }
+
+        public MigrationTransient Transient { get; }
+
+        public IStartupLogger<TestMigration> Logger { get; }
+
+        public Task PerformAsync(CancellationToken cancellationToken)
+        {
+            Performed = this;
+            return Task.CompletedTask;
+        }
+    }
+}
diff --git a/tests/Jellyfin.Server.Tests/ServerSetupApp/StartupLoggerTests.cs b/tests/Jellyfin.Server.Tests/ServerSetupApp/StartupLoggerTests.cs
new file mode 100644
index 0000000000..c2894e9647
--- /dev/null
+++ b/tests/Jellyfin.Server.Tests/ServerSetupApp/StartupLoggerTests.cs
@@ -0,0 +1,54 @@
+using Jellyfin.Server.ServerSetupApp;
+using Microsoft.Extensions.Logging.Abstractions;
+using Xunit;
+
+namespace Jellyfin.Server.Tests.ServerSetupApp;
+
+public class StartupLoggerTests
+{
+    [Fact]
+    public void BeginAmbientTopic_AttachesNewLoggersToTheTopic()
+    {
+        var migration = new StartupLogger(NullLogger.Instance).BeginGroup($"Migration");
+
+        using (StartupLogger.BeginAmbientTopic(migration.Topic))
+        {
+            Assert.Same(migration.Topic, new StartupLogger(NullLogger.Instance).Topic);
+        }
+    }
+
+    [Fact]
+    public void BeginAmbientTopic_RestoresThePreviousTopic()
+    {
+        var root = new StartupLogger(NullLogger.Instance);
+        var outer = root.BeginGroup($"Outer");
+        var inner = outer.BeginGroup($"Inner");
+
+        Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
+
+        using (StartupLogger.BeginAmbientTopic(outer.Topic))
+        {
+            using (StartupLogger.BeginAmbientTopic(inner.Topic))
+            {
+                Assert.Same(inner.Topic, new StartupLogger(NullLogger.Instance).Topic);
+            }
+
+            // Leaving a nested topic has to fall back to the enclosing one, not to the setup UI root.
+            Assert.Same(outer.Topic, new StartupLogger(NullLogger.Instance).Topic);
+        }
+
+        Assert.Null(new StartupLogger(NullLogger.Instance).Topic);
+    }
+
+    [Fact]
+    public void BeginGroup_KeepsAnExplicitTopicOverTheAmbientOne()
+    {
+        var migration = new StartupLogger(NullLogger.Instance).BeginGroup($"Migration");
+        var unrelated = new StartupLogger(NullLogger.Instance).BeginGroup($"Unrelated");
+
+        using (StartupLogger.BeginAmbientTopic(migration.Topic))
+        {
+            Assert.Same(unrelated.Topic, unrelated.With(NullLogger.Instance).Topic);
+        }
+    }
+}