From c622a16261d8c4e364a05caeecee798c54803501 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sat, 5 Sep 2026 07:23:39 +0200 Subject: Don't dispose application singletons after running a code migration --- Jellyfin.Server/Migrations/Stages/CodeMigration.cs | 120 ++++++++++++++++----- .../Migrations/CodeMigrationTests.cs | 90 ++++++++++++++++ 2 files changed, 181 insertions(+), 29 deletions(-) create mode 100644 tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs diff --git a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs index 971b47608f..69e5cbc375 100644 --- a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs +++ b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs @@ -4,7 +4,6 @@ using System.Threading; using System.Threading.Tasks; using Jellyfin.Server.ServerSetupApp; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; namespace Jellyfin.Server.Migrations.Stages; @@ -22,28 +21,6 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta return Metadata.Order.ToString("yyyyMMddHHmmsss", CultureInfo.InvariantCulture) + "_" + Metadata.Name!; } - private IServiceCollection MigrationServices(IServiceProvider serviceProvider, IStartupLogger logger) - { - var childServiceCollection = new ServiceCollection() - .AddSingleton(serviceProvider) - .AddSingleton(logger) - .AddSingleton(typeof(IStartupLogger<>), typeof(NestedStartupLogger<>)) - .AddSingleton(logger.Topic!); - - foreach (ServiceDescriptor service in serviceProvider.GetRequiredService()) - { - if (service.Lifetime == ServiceLifetime.Singleton && !service.ServiceType.IsGenericTypeDefinition) - { - childServiceCollection.AddSingleton(service.ServiceType, _ => serviceProvider.GetService(service.ServiceType)!); - continue; - } - - childServiceCollection.Add(service); - } - - return childServiceCollection; - } - public async Task Perform(IServiceProvider? serviceProvider, IStartupLogger logger, CancellationToken cancellationToken) { #pragma warning disable CS0618 // Type or member is obsolete @@ -55,10 +32,13 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta } else { - using var migrationServices = MigrationServices(serviceProvider, logger).BuildServiceProvider(); - ((IMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).Perform(); -#pragma warning restore CS0618 // Type or member is obsolete + var migrationServices = new MigrationServiceProvider(serviceProvider, logger); + await using (migrationServices.ConfigureAwait(false)) + { + ((IMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).Perform(); + } } +#pragma warning restore CS0618 // Type or member is obsolete } else if (typeof(IAsyncMigrationRoutine).IsAssignableFrom(MigrationType)) { @@ -68,8 +48,11 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta } else { - using var migrationServices = MigrationServices(serviceProvider, logger).BuildServiceProvider(); - await ((IAsyncMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).PerformAsync(cancellationToken).ConfigureAwait(false); + var migrationServices = new MigrationServiceProvider(serviceProvider, logger); + await using (migrationServices.ConfigureAwait(false)) + { + await ((IAsyncMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).PerformAsync(cancellationToken).ConfigureAwait(false); + } } } else @@ -78,9 +61,88 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta } } + /// + /// Provides the services a migration routine is constructed with. + /// + /// + /// This overlays the migration scoped logging services onto a scope of the application container. Copying the + /// application service descriptors into a child container instead would make that child container the owner of + /// every singleton it forwards, so disposing it after the migration would also dispose the applications own + /// instance of services like the ProviderManager and leave the server broken until the next restart. + /// + private sealed class MigrationServiceProvider : IServiceProvider, IServiceProviderIsService, IAsyncDisposable + { + private readonly AsyncServiceScope _scope; + private readonly IStartupLogger _logger; + private readonly IServiceProviderIsService? _isService; + + public MigrationServiceProvider(IServiceProvider serviceProvider, IStartupLogger logger) + { + _scope = serviceProvider.CreateAsyncScope(); + _logger = logger; + _isService = _scope.ServiceProvider.GetService(); + } + + public object? GetService(Type serviceType) + { + if (serviceType == typeof(IServiceProvider)) + { + return this; + } + + if (serviceType == typeof(IServiceProviderIsService)) + { + return _isService is null ? null : this; + } + + if (serviceType == typeof(IStartupLogger)) + { + return _logger; + } + + if (serviceType == typeof(StartupLogTopic)) + { + return _logger.Topic; + } + + if (IsCategoryLogger(serviceType)) + { + var category = serviceType.GenericTypeArguments[0]; + var baseLogger = _scope.ServiceProvider.GetRequiredService(typeof(ILogger<>).MakeGenericType(category)); + return Activator.CreateInstance(typeof(NestedStartupLogger<>).MakeGenericType(category), baseLogger, _logger.Topic); + } + + return _scope.ServiceProvider.GetService(serviceType); + } + + public bool IsService(Type serviceType) + { + if (serviceType == typeof(IServiceProvider) + || serviceType == typeof(IServiceProviderIsService) + || serviceType == typeof(IStartupLogger) + || serviceType == typeof(StartupLogTopic) + || IsCategoryLogger(serviceType)) + { + return true; + } + + return _isService?.IsService(serviceType) ?? false; + } + + public ValueTask DisposeAsync() + { + return _scope.DisposeAsync(); + } + + private static bool IsCategoryLogger(Type serviceType) + { + return serviceType.IsConstructedGenericType && serviceType.GetGenericTypeDefinition() == typeof(IStartupLogger<>); + } + } + private class NestedStartupLogger : StartupLogger { - public NestedStartupLogger(ILogger logger, StartupLogTopic topic) : base(logger, topic) + public NestedStartupLogger(ILogger logger, StartupLogTopic? topic) : base(logger, topic) { } } diff --git a/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs new file mode 100644 index 0000000000..68dd4486be --- /dev/null +++ b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs @@ -0,0 +1,90 @@ +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() + .AddTransient(); + services.AddSingleton(services); + + await using var serviceProvider = services.BuildServiceProvider(); + var applicationSingleton = serviceProvider.GetRequiredService(); + 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()); + // 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); + } + + 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 logger) + { + Singleton = singleton; + Transient = transient; + Logger = logger; + } + + public static TestMigration? Performed { get; private set; } + + public ApplicationSingleton Singleton { get; } + + public MigrationTransient Transient { get; } + + public IStartupLogger Logger { get; } + + public Task PerformAsync(CancellationToken cancellationToken) + { + Performed = this; + return Task.CompletedTask; + } + } +} -- cgit v1.2.3 From 9c259027dfa6d799dab282fd2ee0315e572b4a7c Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sat, 5 Sep 2026 08:55:12 +0200 Subject: Cleanup --- Jellyfin.Server/Program.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 12f92efb35..26731d65bd 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -180,9 +180,7 @@ namespace Jellyfin.Server }) .ConfigureAppConfiguration(config => config.ConfigureAppConfiguration(options, appPaths, startupConfig)) .UseSerilog() - .ConfigureServices(e => e - .RegisterStartupLogger() - .AddSingleton(e)) + .ConfigureServices(e => e.RegisterStartupLogger()) .Build(); /* @@ -307,7 +305,6 @@ namespace Jellyfin.Server .AddSingleton(appPaths) .RegisterStartupLogger(); - migrationStartupServiceProvider.AddSingleton(migrationStartupServiceProvider); var startupService = migrationStartupServiceProvider.BuildServiceProvider(); PrepareDatabaseProvider(startupService); -- cgit v1.2.3 From ca90347dc242e7dc5e49e6fb8938e084564ab2de Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 6 Sep 2026 07:46:18 +0200 Subject: Resolve migration routine loggers from the application container --- .../20250618010000_MigrateLibraryUserData.cs | 2 +- .../Routines/20250730215000_ReseedFolderFlag.cs | 2 +- Jellyfin.Server/Migrations/Stages/CodeMigration.cs | 133 +++++---------------- Jellyfin.Server/ServerSetupApp/StartupLogger.cs | 31 +++++ .../Migrations/CodeMigrationTests.cs | 24 +++- .../ServerSetupApp/StartupLoggerTests.cs | 54 +++++++++ 6 files changed, 137 insertions(+), 109 deletions(-) create mode 100644 tests/Jellyfin.Server.Tests/ServerSetupApp/StartupLoggerTests.cs diff --git a/Jellyfin.Server/Migrations/Routines/20250618010000_MigrateLibraryUserData.cs b/Jellyfin.Server/Migrations/Routines/20250618010000_MigrateLibraryUserData.cs index 8a0a1741f1..291de23b2e 100644 --- a/Jellyfin.Server/Migrations/Routines/20250618010000_MigrateLibraryUserData.cs +++ b/Jellyfin.Server/Migrations/Routines/20250618010000_MigrateLibraryUserData.cs @@ -29,7 +29,7 @@ internal class MigrateLibraryUserData : IAsyncMigrationRoutine private readonly IDbContextFactory _provider; public MigrateLibraryUserData( - IStartupLogger startupLogger, + IStartupLogger startupLogger, IDbContextFactory provider, IServerApplicationPaths paths) { diff --git a/Jellyfin.Server/Migrations/Routines/20250730215000_ReseedFolderFlag.cs b/Jellyfin.Server/Migrations/Routines/20250730215000_ReseedFolderFlag.cs index 502763ac09..c8ee44a670 100644 --- a/Jellyfin.Server/Migrations/Routines/20250730215000_ReseedFolderFlag.cs +++ b/Jellyfin.Server/Migrations/Routines/20250730215000_ReseedFolderFlag.cs @@ -24,7 +24,7 @@ internal class ReseedFolderFlag : IAsyncMigrationRoutine private readonly IDbContextFactory _provider; public ReseedFolderFlag( - IStartupLogger startupLogger, + IStartupLogger startupLogger, IDbContextFactory provider, IServerApplicationPaths paths) { diff --git a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs index 69e5cbc375..30e9f1d3d0 100644 --- a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs +++ b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs @@ -4,7 +4,6 @@ using System.Threading; using System.Threading.Tasks; using Jellyfin.Server.ServerSetupApp; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; namespace Jellyfin.Server.Migrations.Stages; @@ -23,127 +22,49 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta public async Task Perform(IServiceProvider? serviceProvider, IStartupLogger logger, CancellationToken cancellationToken) { -#pragma warning disable CS0618 // Type or member is obsolete - if (typeof(IMigrationRoutine).IsAssignableFrom(MigrationType)) - { - if (serviceProvider is null) - { - ((IMigrationRoutine)Activator.CreateInstance(MigrationType)!).Perform(); - } - else - { - var migrationServices = new MigrationServiceProvider(serviceProvider, logger); - await using (migrationServices.ConfigureAwait(false)) - { - ((IMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).Perform(); - } - } -#pragma warning restore CS0618 // Type or member is obsolete - } - else if (typeof(IAsyncMigrationRoutine).IsAssignableFrom(MigrationType)) - { - if (serviceProvider is null) - { - await ((IAsyncMigrationRoutine)Activator.CreateInstance(MigrationType)!).PerformAsync(cancellationToken).ConfigureAwait(false); - } - else - { - var migrationServices = new MigrationServiceProvider(serviceProvider, logger); - await using (migrationServices.ConfigureAwait(false)) - { - await ((IAsyncMigrationRoutine)ActivatorUtilities.CreateInstance(migrationServices, MigrationType)).PerformAsync(cancellationToken).ConfigureAwait(false); - } - } - } - else + if (!IsMigrationRoutine(MigrationType)) { throw new InvalidOperationException($"The type {MigrationType} does not implement either IMigrationRoutine or IAsyncMigrationRoutine and is not a valid migration type"); } - } - /// - /// Provides the services a migration routine is constructed with. - /// - /// - /// This overlays the migration scoped logging services onto a scope of the application container. Copying the - /// application service descriptors into a child container instead would make that child container the owner of - /// every singleton it forwards, so disposing it after the migration would also dispose the applications own - /// instance of services like the ProviderManager and leave the server broken until the next restart. - /// - private sealed class MigrationServiceProvider : IServiceProvider, IServiceProviderIsService, IAsyncDisposable - { - private readonly AsyncServiceScope _scope; - private readonly IStartupLogger _logger; - private readonly IServiceProviderIsService? _isService; - - public MigrationServiceProvider(IServiceProvider serviceProvider, IStartupLogger logger) + if (serviceProvider is null) { - _scope = serviceProvider.CreateAsyncScope(); - _logger = logger; - _isService = _scope.ServiceProvider.GetService(); - } - - public object? GetService(Type serviceType) - { - if (serviceType == typeof(IServiceProvider)) - { - return this; - } - - if (serviceType == typeof(IServiceProviderIsService)) - { - return _isService is null ? null : this; - } - - if (serviceType == typeof(IStartupLogger)) - { - return _logger; - } - - if (serviceType == typeof(StartupLogTopic)) - { - return _logger.Topic; - } - - if (IsCategoryLogger(serviceType)) - { - var category = serviceType.GenericTypeArguments[0]; - var baseLogger = _scope.ServiceProvider.GetRequiredService(typeof(ILogger<>).MakeGenericType(category)); - return Activator.CreateInstance(typeof(NestedStartupLogger<>).MakeGenericType(category), baseLogger, _logger.Topic); - } - - return _scope.ServiceProvider.GetService(serviceType); + await RunAsync(Activator.CreateInstance(MigrationType)!, cancellationToken).ConfigureAwait(false); + return; } - public bool IsService(Type serviceType) + // The routine runs against a scope of the applications own container. Copying the application service + // descriptors into a child container instead would make that child container the owner of every singleton it + // forwards, so disposing it after the migration would also dispose the applications own instance of services + // like the ProviderManager and leave the server broken until the next restart. + var scope = serviceProvider.CreateAsyncScope(); + await using (scope.ConfigureAwait(false)) { - if (serviceType == typeof(IServiceProvider) - || serviceType == typeof(IServiceProviderIsService) - || serviceType == typeof(IStartupLogger) - || serviceType == typeof(StartupLogTopic) - || IsCategoryLogger(serviceType)) + // Nests everything the routine logs through an injected IStartupLogger under the migrations own topic. + using (StartupLogger.BeginAmbientTopic(logger.Topic)) { - return true; + await RunAsync(ActivatorUtilities.CreateInstance(scope.ServiceProvider, MigrationType), cancellationToken).ConfigureAwait(false); } - - return _isService?.IsService(serviceType) ?? false; - } - - public ValueTask DisposeAsync() - { - return _scope.DisposeAsync(); } + } - private static bool IsCategoryLogger(Type serviceType) - { - return serviceType.IsConstructedGenericType && serviceType.GetGenericTypeDefinition() == typeof(IStartupLogger<>); - } + // The obsolete IMigrationRoutine is still implemented by every routine that predates the async interface, so + // the members that have to touch it are grouped here behind a single suppression. +#pragma warning disable CS0618 // Type or member is obsolete + private static bool IsMigrationRoutine(Type migrationType) + { + return typeof(IMigrationRoutine).IsAssignableFrom(migrationType) || typeof(IAsyncMigrationRoutine).IsAssignableFrom(migrationType); } - private class NestedStartupLogger : StartupLogger + private static async Task RunAsync(object routine, CancellationToken cancellationToken) { - public NestedStartupLogger(ILogger logger, StartupLogTopic? topic) : base(logger, topic) + if (routine is IMigrationRoutine migrationRoutine) { + migrationRoutine.Perform(); + return; } + + await ((IAsyncMigrationRoutine)routine).PerformAsync(cancellationToken).ConfigureAwait(false); } +#pragma warning restore CS0618 // Type or member is obsolete } diff --git a/Jellyfin.Server/ServerSetupApp/StartupLogger.cs b/Jellyfin.Server/ServerSetupApp/StartupLogger.cs index 0121854ce3..b72b0c0eab 100644 --- a/Jellyfin.Server/ServerSetupApp/StartupLogger.cs +++ b/Jellyfin.Server/ServerSetupApp/StartupLogger.cs @@ -1,5 +1,6 @@ using System; using System.Globalization; +using System.Threading; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -8,6 +9,8 @@ namespace Jellyfin.Server.ServerSetupApp; /// public class StartupLogger : IStartupLogger { + private static readonly AsyncLocal _ambientTopic = new(); + private readonly StartupLogTopic? _topic; /// @@ -17,6 +20,7 @@ public class StartupLogger : IStartupLogger public StartupLogger(ILogger logger) { BaseLogger = logger; + _topic = _ambientTopic.Value; } /// @@ -39,6 +43,18 @@ public class StartupLogger : IStartupLogger /// protected ILogger BaseLogger { get; set; } + /// + /// Makes the topic that loggers created on this execution context attach to. + /// + /// The topic to nest newly created loggers under. + /// A scope that restores the previously ambient topic when disposed. + internal static IDisposable BeginAmbientTopic(StartupLogTopic? topic) + { + var scope = new AmbientTopicScope(_ambientTopic.Value); + _ambientTopic.Value = topic; + return scope; + } + /// public IStartupLogger BeginGroup(FormattableString logEntry) { @@ -121,4 +137,19 @@ public class StartupLogger : IStartupLogger Topic.Children.Add(startupEntry); } } + + private sealed class AmbientTopicScope : IDisposable + { + private readonly StartupLogTopic? _previous; + + public AmbientTopicScope(StartupLogTopic? previous) + { + _previous = previous; + } + + public void Dispose() + { + _ambientTopic.Value = _previous; + } + } } diff --git a/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs index 68dd4486be..3bd8581a5f 100644 --- a/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs +++ b/tests/Jellyfin.Server.Tests/Migrations/CodeMigrationTests.cs @@ -20,7 +20,6 @@ public class CodeMigrationTests .RegisterStartupLogger() .AddSingleton() .AddTransient(); - services.AddSingleton(services); await using var serviceProvider = services.BuildServiceProvider(); var applicationSingleton = serviceProvider.GetRequiredService(); @@ -44,6 +43,29 @@ public class CodeMigrationTests Assert.Same(logger.Topic, performed.Logger.Topic); } + [Fact] + public async Task Perform_DoesNotLeakTheMigrationTopic() + { + var services = new ServiceCollection() + .AddLogging() + .RegisterStartupLogger() + .AddSingleton() + .AddTransient(); + + 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>().Topic); + Assert.Null(new StartupLogger(NullLogger.Instance).Topic); + } + private sealed class ApplicationSingleton : IDisposable { public bool IsDisposed { get; private set; } 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); + } + } +} -- cgit v1.2.3 From 63553803b19446ea9b5cb9b335015ab8727a3799 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 6 Sep 2026 09:49:26 +0200 Subject: Remove the unreachable null service provider path from code migrations --- Jellyfin.Server/Migrations/JellyfinMigrationService.cs | 6 +++--- Jellyfin.Server/Migrations/Stages/CodeMigration.cs | 8 +------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs index beafc3916f..1299896cb9 100644 --- a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs +++ b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs @@ -183,7 +183,7 @@ internal class JellyfinMigrationService } } - public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) + public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider serviceProvider) { var logger = _startupLogger.With(_loggerFactory.CreateLogger()).BeginGroup($"Migrate stage {stage}."); ICollection migrationStage = (Migrations.FirstOrDefault(e => e.Stage == stage) as ICollection) ?? []; @@ -445,10 +445,10 @@ internal class JellyfinMigrationService private class InternalCodeMigration : IInternalMigration { private readonly CodeMigration _codeMigration; - private readonly IServiceProvider? _serviceProvider; + private readonly IServiceProvider _serviceProvider; private JellyfinDbContext _dbContext; - public InternalCodeMigration(CodeMigration codeMigration, IServiceProvider? serviceProvider, JellyfinDbContext dbContext) + public InternalCodeMigration(CodeMigration codeMigration, IServiceProvider serviceProvider, JellyfinDbContext dbContext) { _codeMigration = codeMigration; _serviceProvider = serviceProvider; diff --git a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs index 30e9f1d3d0..71706811b8 100644 --- a/Jellyfin.Server/Migrations/Stages/CodeMigration.cs +++ b/Jellyfin.Server/Migrations/Stages/CodeMigration.cs @@ -20,19 +20,13 @@ internal class CodeMigration(Type migrationType, JellyfinMigrationAttribute meta return Metadata.Order.ToString("yyyyMMddHHmmsss", CultureInfo.InvariantCulture) + "_" + Metadata.Name!; } - public async Task Perform(IServiceProvider? serviceProvider, IStartupLogger logger, CancellationToken cancellationToken) + public async Task Perform(IServiceProvider serviceProvider, IStartupLogger logger, CancellationToken cancellationToken) { if (!IsMigrationRoutine(MigrationType)) { throw new InvalidOperationException($"The type {MigrationType} does not implement either IMigrationRoutine or IAsyncMigrationRoutine and is not a valid migration type"); } - if (serviceProvider is null) - { - await RunAsync(Activator.CreateInstance(MigrationType)!, cancellationToken).ConfigureAwait(false); - return; - } - // The routine runs against a scope of the applications own container. Copying the application service // descriptors into a child container instead would make that child container the owner of every singleton it // forwards, so disposing it after the migration would also dispose the applications own instance of services -- cgit v1.2.3