From fa07a3abe89b6e0eb96a9f8d8a3eb57dea20ca2a Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Fri, 26 Jun 2026 07:34:19 +0200 Subject: Skip backups whens can is running --- .../FullSystemBackup/BackupService.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs') diff --git a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs index a6dc5458ee..a534fa5fa0 100644 --- a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs +++ b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs @@ -12,6 +12,7 @@ using Jellyfin.Database.Implementations; using Jellyfin.Server.Implementations.StorageHelpers; using Jellyfin.Server.Implementations.SystemBackupService; using MediaBrowser.Controller; +using MediaBrowser.Controller.Library; using MediaBrowser.Controller.SystemBackupService; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -33,6 +34,7 @@ public class BackupService : IBackupService private readonly IServerApplicationPaths _applicationPaths; private readonly IJellyfinDatabaseProvider _jellyfinDatabaseProvider; private readonly IHostApplicationLifetime _hostApplicationLifetime; + private readonly ILibraryManager _libraryManager; private static readonly JsonSerializerOptions _serializerSettings = new JsonSerializerOptions(JsonSerializerDefaults.General) { AllowTrailingCommas = true, @@ -50,13 +52,15 @@ public class BackupService : IBackupService /// The application paths. /// The Jellyfin database Provider in use. /// The SystemManager. + /// Instance of the interface. public BackupService( ILogger logger, IDbContextFactory dbProvider, IServerApplicationHost applicationHost, IServerApplicationPaths applicationPaths, IJellyfinDatabaseProvider jellyfinDatabaseProvider, - IHostApplicationLifetime applicationLifetime) + IHostApplicationLifetime applicationLifetime, + ILibraryManager libraryManager) { _logger = logger; _dbProvider = dbProvider; @@ -64,6 +68,7 @@ public class BackupService : IBackupService _applicationPaths = applicationPaths; _jellyfinDatabaseProvider = jellyfinDatabaseProvider; _hostApplicationLifetime = applicationLifetime; + _libraryManager = libraryManager; } /// @@ -263,6 +268,14 @@ public class BackupService : IBackupService /// public async Task CreateBackupAsync(BackupOptionsDto backupOptions) { + // Creating a backup runs a database optimization and reads the entire database under a transaction, both of + // which heavily contend with an active library scan and could capture an inconsistent database state. + if (_libraryManager.IsScanRunning) + { + _logger.LogWarning("Cannot create a backup while a library scan is running."); + throw new InvalidOperationException("Cannot create a backup while a library scan is running. Please try again once the scan has finished."); + } + var manifest = new BackupManifest() { DateCreated = DateTime.UtcNow, -- cgit v1.2.3 From d7727224c2f5024c9981bc70a274826beb7f1cdf Mon Sep 17 00:00:00 2001 From: zerafachris Date: Fri, 17 Jul 2026 16:44:25 +0200 Subject: Skip corrupt KeyframeData rows during full system backup A single row with malformed KeyframeTicks JSON (e.g. a truncated array from an interrupted write) currently aborts the entire backup, because the try/catch in BackupService.CreateBackupAsync only wraps serialization of an already-materialized entity, not the enumeration itself. EF Core throws JsonReaderException from MoveNextAsync() while materializing the corrupt row, which propagates past that catch block. Switch to manual enumerator iteration so MoveNextAsync() failures can be caught per-row, logged as a warning identifying the affected table, and skipped, allowing the remaining rows and the rest of the backup to complete. Fixes #17216 Co-Authored-By: Claude Sonnet 5 --- .../FullSystemBackup/BackupService.cs | 44 +++-- .../FullSystemBackup/BackupServiceTests.cs | 178 +++++++++++++++++++++ 2 files changed, 212 insertions(+), 10 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs (limited to 'Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs') diff --git a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs index a534fa5fa0..16daba0991 100644 --- a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs +++ b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs @@ -359,18 +359,42 @@ public class BackupService : IBackupService jsonSerializer.WriteStartArray(); var set = entityType.ValueFactory().ConfigureAwait(false); - await foreach (var item in set.ConfigureAwait(false)) + var enumerator = set.GetAsyncEnumerator(); + await using (enumerator) { - entities++; - try + while (true) { - using var document = JsonSerializer.SerializeToDocument(item, _serializerSettings); - document.WriteTo(jsonSerializer); - } - catch (Exception ex) - { - _logger.LogError(ex, "Could not load entity {Entity}", item); - throw; + // Reading the next row can itself throw, e.g. when a column contains malformed + // JSON (see https://github.com/jellyfin/jellyfin/issues/17216). Catch that here so a single + // corrupt row is skipped, logged for manual follow-up, and does not abort the whole backup. + bool hasNext; + try + { + hasNext = await enumerator.MoveNextAsync(); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not read next entity of type {Table}, the underlying data appears to be corrupt. Skipping this row and continuing backup; the affected database row should be inspected and fixed manually", entityType.SourceName); + continue; + } + + if (!hasNext) + { + break; + } + + var item = enumerator.Current; + entities++; + try + { + using var document = JsonSerializer.SerializeToDocument(item, _serializerSettings); + document.WriteTo(jsonSerializer); + } + catch (Exception ex) + { + _logger.LogError(ex, "Could not load entity {Entity}", item); + throw; + } } } diff --git a/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs b/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs new file mode 100644 index 0000000000..e868b8c9a5 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/FullSystemBackup/BackupServiceTests.cs @@ -0,0 +1,178 @@ +using System; +using System.IO; +using System.IO.Compression; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.Sqlite; +using Jellyfin.Server.Implementations.FullSystemBackup; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.SystemBackupService; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; +using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind; + +namespace Jellyfin.Server.Implementations.Tests.FullSystemBackup; + +/// +/// Tests for , in particular that a single row of corrupt +/// (e.g. malformed KeyframeTicks JSON) does not abort +/// an otherwise healthy backup. See https://github.com/jellyfin/jellyfin/issues/17216. +/// +public sealed class BackupServiceTests : IDisposable +{ + private readonly SqliteConnection _connection; + private readonly DbContextOptions _dbOptions; + private readonly string _testRoot; + private readonly string _backupPath; + private readonly string _configurationDirectoryPath; + + public BackupServiceTests() + { + _connection = new SqliteConnection("Data Source=:memory:"); + _connection.Open(); + + _dbOptions = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .Options; + + using (var ctx = CreateDbContext()) + { + ctx.Database.EnsureCreated(); + } + + _testRoot = Path.Combine(Path.GetTempPath(), "jellyfin-backup-service-tests-" + Guid.NewGuid().ToString("N")); + _backupPath = Path.Combine(_testRoot, "Backup"); + _configurationDirectoryPath = Path.Combine(_testRoot, "Config"); + Directory.CreateDirectory(_backupPath); + Directory.CreateDirectory(_configurationDirectoryPath); + } + + public void Dispose() + { + _connection.Dispose(); + + if (Directory.Exists(_testRoot)) + { + Directory.Delete(_testRoot, true); + } + } + + [Fact] + public async Task CreateBackupAsync_WithCorruptKeyframeDataRow_SkipsRowAndCompletesBackup() + { + var cancellationToken = TestContext.Current.CancellationToken; + var validItemId = Guid.Parse("11111111-1111-1111-1111-111111111111"); + var corruptItemId = Guid.Parse("22222222-2222-2222-2222-222222222222"); + + await using (var ctx = CreateDbContext()) + { + // A healthy item + keyframe row, written the normal way. + ctx.BaseItems.Add(CreateMovieEntity(validItemId, "Good Movie")); + ctx.BaseItems.Add(CreateMovieEntity(corruptItemId, "Corrupt Movie")); + await ctx.SaveChangesAsync(cancellationToken).ConfigureAwait(true); + + ctx.KeyframeData.Add(new KeyframeData + { + ItemId = validItemId, + TotalDuration = 60_000, + KeyframeTicks = [0, 1000, 2000] + }); + await ctx.SaveChangesAsync(cancellationToken).ConfigureAwait(true); + + // Simulate a corrupted database row: truncated JSON array for KeyframeTicks, + // written directly via SQL to bypass EF's normal (well-formed) write path. + await ctx.Database.ExecuteSqlInterpolatedAsync( + $"INSERT INTO KeyframeData (ItemId, TotalDuration, KeyframeTicks) VALUES ({corruptItemId.ToString()}, {5000L}, {"[1,2,3"})", + cancellationToken).ConfigureAwait(true); + } + + var backupService = CreateBackupService(); + + var manifest = await backupService.CreateBackupAsync(new BackupOptionsDto()).ConfigureAwait(true); + + Assert.True(File.Exists(manifest.Path)); + + using var archive = await ZipFile.OpenReadAsync(manifest.Path, cancellationToken).ConfigureAwait(true); + var keyframeEntry = archive.GetEntry("Database/KeyframeData.json"); + Assert.NotNull(keyframeEntry); + + await using var entryStream = await keyframeEntry!.OpenAsync(cancellationToken).ConfigureAwait(true); + using var document = await JsonDocument.ParseAsync(entryStream, cancellationToken: cancellationToken).ConfigureAwait(true); + + var rows = document.RootElement.EnumerateArray().ToList(); + + // The corrupt row must be skipped, but the valid row must still make it into the backup. + var singleRow = Assert.Single(rows); + Assert.Equal(validItemId, singleRow.GetProperty("ItemId").GetGuid()); + } + + private BackupService CreateBackupService() + { + var factory = new Mock>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny())).ReturnsAsync(CreateDbContext); + + var applicationHost = new Mock(); + applicationHost.Setup(a => a.ApplicationVersion).Returns(new Version(10, 11, 0)); + + var applicationPaths = new Mock(); + applicationPaths.Setup(a => a.BackupPath).Returns(_backupPath); + applicationPaths.Setup(a => a.ConfigurationDirectoryPath).Returns(_configurationDirectoryPath); + applicationPaths.Setup(a => a.DataPath).Returns(Path.Combine(_testRoot, "Data")); + applicationPaths.Setup(a => a.RootFolderPath).Returns(Path.Combine(_testRoot, "Root")); + applicationPaths.Setup(a => a.InternalMetadataPath).Returns(Path.Combine(_testRoot, "Metadata")); + applicationPaths.Setup(a => a.DefaultInternalMetadataPath).Returns(Path.Combine(_testRoot, "MetadataDefault")); + + var jellyfinDatabaseProvider = new Mock(); + jellyfinDatabaseProvider.Setup(p => p.RunScheduledOptimisation(It.IsAny())).Returns(Task.CompletedTask); + jellyfinDatabaseProvider.Setup(p => p.PurgeDatabase(It.IsAny(), It.IsAny>())).Returns(Task.CompletedTask); + + var applicationLifetime = new Mock(); + + var libraryManager = new Mock(); + libraryManager.Setup(l => l.IsScanRunning).Returns(false); + + return new BackupService( + NullLogger.Instance, + factory.Object, + applicationHost.Object, + applicationPaths.Object, + jellyfinDatabaseProvider.Object, + applicationLifetime.Object, + libraryManager.Object); + } + + private static BaseItemEntity CreateMovieEntity(Guid id, string name) + { + return new BaseItemEntity + { + Id = id, + Type = "Movie", + Name = name, + PresentationUniqueKey = id.ToString("N"), + MediaType = "Video", + IsMovie = true, + IsFolder = false, + IsVirtualItem = false + }; + } + + private JellyfinDbContext CreateDbContext() + { + return new JellyfinDbContext( + _dbOptions, + NullLogger.Instance, + new SqliteDatabaseProvider(null!, NullLogger.Instance), + new NoLockBehavior(NullLogger.Instance)); + } +} -- cgit v1.2.3 From 663a873e07b436cd99fa7b00efa2bcd68e11fb23 Mon Sep 17 00:00:00 2001 From: zerafachris Date: Sat, 18 Jul 2026 16:47:23 +0200 Subject: fix: log corrupt KeyframeData row read failures as errors, not warnings Per review feedback from cvium: failing to read/backup an entity due to corrupt underlying data is a significant event that should be surfaced as an error, not silently downgraded to a warning. --- Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs') diff --git a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs index 16daba0991..765f8bfb73 100644 --- a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs +++ b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs @@ -374,7 +374,7 @@ public class BackupService : IBackupService } catch (Exception ex) { - _logger.LogWarning(ex, "Could not read next entity of type {Table}, the underlying data appears to be corrupt. Skipping this row and continuing backup; the affected database row should be inspected and fixed manually", entityType.SourceName); + _logger.LogError(ex, "Could not read next entity of type {Table}, the underlying data appears to be corrupt. Skipping this row and continuing backup; the affected database row should be inspected and fixed manually", entityType.SourceName); continue; } -- cgit v1.2.3 From 0d629591ed8b491e6e3560f72b12d737e4f3922f Mon Sep 17 00:00:00 2001 From: Cody Robibero Date: Mon, 20 Jul 2026 20:15:05 -0400 Subject: Remove comments about JSON error handling Removed comments explaining error handling for malformed JSON during backup. --- Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs | 3 --- 1 file changed, 3 deletions(-) (limited to 'Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs') diff --git a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs index 765f8bfb73..7c10a5dc77 100644 --- a/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs +++ b/Jellyfin.Server.Implementations/FullSystemBackup/BackupService.cs @@ -364,9 +364,6 @@ public class BackupService : IBackupService { while (true) { - // Reading the next row can itself throw, e.g. when a column contains malformed - // JSON (see https://github.com/jellyfin/jellyfin/issues/17216). Catch that here so a single - // corrupt row is skipped, logged for manual follow-up, and does not abort the whole backup. bool hasNext; try { -- cgit v1.2.3