From fa8ae9c432af66e8d840854e50bafb1baf7efaba Mon Sep 17 00:00:00 2001 From: William W Date: Fri, 21 Aug 2026 21:59:12 -0700 Subject: [PATCH] Fix TorrentRunner infinite crash loop on duplicate download path (#1033) --- server/RdtClient.Data/Data/DownloadData.cs | 45 +++- .../Regression/DownloadPathCollisionTests.cs | 205 ++++++++++++++++++ .../BackgroundServices/TaskRunner.cs | 27 ++- .../RdtClient.Service/Services/Downloads.cs | 4 +- .../RdtClient.Service/Services/IDownloads.cs | 2 +- .../Services/TorrentRunner.cs | 41 ++-- server/RdtClient.Service/Services/Torrents.cs | 8 +- 7 files changed, 310 insertions(+), 22 deletions(-) create mode 100644 server/RdtClient.Service.Test/Regression/DownloadPathCollisionTests.cs diff --git a/server/RdtClient.Data/Data/DownloadData.cs b/server/RdtClient.Data/Data/DownloadData.cs index d39400ea..49dc6d51 100644 --- a/server/RdtClient.Data/Data/DownloadData.cs +++ b/server/RdtClient.Data/Data/DownloadData.cs @@ -130,19 +130,58 @@ public async Task UpdateFileName(Guid downloadId, String fileName) await dataContext.SaveChangesAsync(); } - public async Task UpdatePath(Guid downloadId, String path) + public async Task UpdatePath(Guid downloadId, String path) { var dbDownload = await dataContext.Downloads .FirstOrDefaultAsync(m => m.DownloadId == downloadId); if (dbDownload == null) { - return; + return false; + } + + if (String.Equals(dbDownload.Path, path, StringComparison.Ordinal)) + { + return true; + } + + var pathAlreadyTaken = await dataContext.Downloads + .AsNoTracking() + .AnyAsync(m => m.TorrentId == dbDownload.TorrentId + && m.DownloadId != downloadId + && m.Path == path); + + if (pathAlreadyTaken) + { + logger?.LogWarning("Skipped updating path because another download of the same torrent already uses it. DownloadId: {downloadId}, TorrentId: {torrentId}, Path: {path}", + downloadId, + dbDownload.TorrentId, + path); + + return false; } dbDownload.Path = path; - await dataContext.SaveChangesAsync(); + try + { + await dataContext.SaveChangesAsync(); + } + catch (DbUpdateException ex) + { + dataContext.Entry(dbDownload).State = EntityState.Detached; + + if (IsDuplicateDownloadViolation(ex)) + { + logger?.LogWarning("Skipped updating path after a concurrent duplicate path write. DownloadId: {downloadId}, Path: {path}", downloadId, path); + + return false; + } + + throw; + } + + return true; } public async Task UpdateDownloadStarted(Guid downloadId, DateTimeOffset? dateTime) diff --git a/server/RdtClient.Service.Test/Regression/DownloadPathCollisionTests.cs b/server/RdtClient.Service.Test/Regression/DownloadPathCollisionTests.cs new file mode 100644 index 00000000..3f457e71 --- /dev/null +++ b/server/RdtClient.Service.Test/Regression/DownloadPathCollisionTests.cs @@ -0,0 +1,205 @@ +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using RdtClient.Data.Data; +using RdtClient.Data.Models.Data; + +namespace RdtClient.Service.Test.Regression; + +public class DownloadPathCollisionTests : IAsyncLifetime +{ + private readonly String _databasePath = Path.Combine(Path.GetTempPath(), $"rdt-client-path-collision-{Guid.NewGuid():N}.sqlite"); + + [Fact] + public async Task UpdatePath_WhenSiblingAlreadyUsesPath_SkipsWriteAndKeepsOriginalPath() + { + var torrentId = await SeedTorrentWithDownloadsAsync(); + + await using var context = CreateContext(); + + var downloadData = new DownloadData(context); + + var downloadB = await context.Downloads.AsNoTracking().FirstAsync(m => m.TorrentId == torrentId && m.Path == "https://example.invalid/b"); + + var updated = await downloadData.UpdatePath(downloadB.DownloadId, "https://example.invalid/a"); + + Assert.False(updated); + + await using var verifyContext = CreateContext(); + + var reloaded = await verifyContext.Downloads.AsNoTracking().FirstAsync(m => m.DownloadId == downloadB.DownloadId); + + Assert.Equal("https://example.invalid/b", reloaded.Path); + } + + [Fact] + public async Task UpdatePath_WhenSiblingUsesPath_DoesNotPoisonTheChangeTracker() + { + var torrentId = await SeedTorrentWithDownloadsAsync(); + + await using var context = CreateContext(); + + var downloadData = new DownloadData(context); + + var downloads = await context.Downloads.AsNoTracking() + .Where(m => m.TorrentId == torrentId) + .OrderBy(m => m.Path) + .ToListAsync(); + + var updated = await downloadData.UpdatePath(downloads[1].DownloadId, "https://example.invalid/a"); + + Assert.False(updated); + + var completedAt = DateTimeOffset.UtcNow; + + Exception? exception; + + try + { + await downloadData.UpdateCompleted(downloads[1].DownloadId, completedAt); + + exception = null; + } + catch (Exception ex) + { + exception = ex; + } + + Assert.Null(exception); + + await using var verifyContext = CreateContext(); + + var reloaded = await verifyContext.Downloads.AsNoTracking().FirstAsync(m => m.DownloadId == downloads[1].DownloadId); + + Assert.Equal("https://example.invalid/b", reloaded.Path); + Assert.NotNull(reloaded.Completed); + } + + [Fact] + public async Task UpdatePath_WhenPathIsFree_UpdatesAndReturnsTrue() + { + var torrentId = await SeedTorrentWithDownloadsAsync(); + + await using var context = CreateContext(); + + var downloadData = new DownloadData(context); + + var downloadB = await context.Downloads.AsNoTracking().FirstAsync(m => m.TorrentId == torrentId && m.Path == "https://example.invalid/b"); + + var updated = await downloadData.UpdatePath(downloadB.DownloadId, "https://example.invalid/refreshed-b"); + + Assert.True(updated); + + await using var verifyContext = CreateContext(); + + var reloaded = await verifyContext.Downloads.AsNoTracking().FirstAsync(m => m.DownloadId == downloadB.DownloadId); + + Assert.Equal("https://example.invalid/refreshed-b", reloaded.Path); + } + + [Fact] + public async Task UpdatePath_WhenPathIsUnchanged_ReturnsTrueWithoutWriting() + { + var torrentId = await SeedTorrentWithDownloadsAsync(); + + await using var context = CreateContext(); + + var downloadData = new DownloadData(context); + + var downloadA = await context.Downloads.AsNoTracking().FirstAsync(m => m.TorrentId == torrentId && m.Path == "https://example.invalid/a"); + + var updated = await downloadData.UpdatePath(downloadA.DownloadId, "https://example.invalid/a"); + + Assert.True(updated); + } + + [Fact] + public async Task SeedSchema_EnforcesUniqueTorrentIdAndPathIndex() + { + var torrentId = await SeedTorrentWithDownloadsAsync(); + + await using var context = CreateContext(); + + var downloadB = await context.Downloads.AsNoTracking().FirstAsync(m => m.TorrentId == torrentId && m.Path == "https://example.invalid/b"); + + context.Downloads.Add(new Download + { + DownloadId = Guid.NewGuid(), + TorrentId = downloadB.TorrentId, + Path = downloadB.Path, + Added = DateTimeOffset.UtcNow + }); + + await Assert.ThrowsAsync(() => context.SaveChangesAsync()); + } + + public async Task InitializeAsync() + { + await using var context = CreateContext(); + await context.Database.EnsureDeletedAsync(); + await context.Database.EnsureCreatedAsync(); + } + + public Task DisposeAsync() + { + SqliteConnection.ClearAllPools(); + + if (File.Exists(_databasePath)) + { + File.Delete(_databasePath); + } + + return Task.CompletedTask; + } + + private DataContext CreateContext() + { + var connectionString = new SqliteConnectionStringBuilder + { + DataSource = _databasePath, + ForeignKeys = true + }.ToString(); + + var options = new DbContextOptionsBuilder() + .UseSqlite(connectionString) + .Options; + + return new(options); + } + + private async Task SeedTorrentWithDownloadsAsync() + { + var torrentId = Guid.NewGuid(); + + await using var context = CreateContext(); + + context.Torrents.Add(new() + { + TorrentId = torrentId, + Hash = Guid.NewGuid().ToString("N"), + Added = DateTimeOffset.UtcNow + }); + + context.Downloads.Add(new() + { + DownloadId = Guid.NewGuid(), + TorrentId = torrentId, + FileName = "download-a.bin", + Path = "https://example.invalid/a", + Added = DateTimeOffset.UtcNow, + Completed = DateTimeOffset.UtcNow + }); + + context.Downloads.Add(new() + { + DownloadId = Guid.NewGuid(), + TorrentId = torrentId, + FileName = "download-b.bin", + Path = "https://example.invalid/b", + Added = DateTimeOffset.UtcNow + }); + + await context.SaveChangesAsync(); + + return torrentId; + } +} diff --git a/server/RdtClient.Service/BackgroundServices/TaskRunner.cs b/server/RdtClient.Service/BackgroundServices/TaskRunner.cs index 52c6c878..7dbc5a3f 100644 --- a/server/RdtClient.Service/BackgroundServices/TaskRunner.cs +++ b/server/RdtClient.Service/BackgroundServices/TaskRunner.cs @@ -24,6 +24,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) await startupRunner.Initialize(); } + var consecutiveFailures = 0; + while (!stoppingToken.IsCancellationRequested) { using var scope = serviceProvider.CreateScope(); @@ -32,6 +34,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) try { await torrentRunner.Tick(); + + consecutiveFailures = 0; } catch (DbUpdateConcurrencyException ex) { @@ -56,12 +60,33 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken) } catch (Exception ex) { + consecutiveFailures++; + logger.LogError(ex, $"Unexpected error occurred in TaskRunner: {ex.Message}"); } - await Task.Delay(TimeSpan.FromSeconds(1), stoppingToken); + var delay = GetTickDelay(consecutiveFailures); + + if (delay > TimeSpan.FromSeconds(1)) + { + logger.LogWarning("TorrentRunner tick failed {consecutiveFailures} times in a row, backing off for {delay}", consecutiveFailures, delay); + } + + await Task.Delay(delay, stoppingToken); } logger.LogInformation("TaskRunner stopped."); } + + private static TimeSpan GetTickDelay(Int32 consecutiveFailures) + { + if (consecutiveFailures <= 1) + { + return TimeSpan.FromSeconds(1); + } + + var seconds = Math.Min(300d, 5d * Math.Pow(2, Math.Min(consecutiveFailures - 2, 6))); + + return TimeSpan.FromSeconds(seconds); + } } diff --git a/server/RdtClient.Service/Services/Downloads.cs b/server/RdtClient.Service/Services/Downloads.cs index d97c9f6f..333ba0ed 100644 --- a/server/RdtClient.Service/Services/Downloads.cs +++ b/server/RdtClient.Service/Services/Downloads.cs @@ -31,9 +31,9 @@ public async Task UpdateUnrestrictedLink(Guid downloadId, String unrestrictedLin await downloadData.UpdateUnrestrictedLink(downloadId, unrestrictedLink); } - public async Task UpdatePath(Guid downloadId, String path) + public async Task UpdatePath(Guid downloadId, String path) { - await downloadData.UpdatePath(downloadId, path); + return await downloadData.UpdatePath(downloadId, path); } public async Task UpdateFileName(Guid downloadId, String fileName) diff --git a/server/RdtClient.Service/Services/IDownloads.cs b/server/RdtClient.Service/Services/IDownloads.cs index 5c5f0c20..99bcad16 100644 --- a/server/RdtClient.Service/Services/IDownloads.cs +++ b/server/RdtClient.Service/Services/IDownloads.cs @@ -10,7 +10,7 @@ public interface IDownloads Task Get(Guid torrentId, String path); Task TryAddForTorrent(Guid torrentId, DownloadInfo downloadInfo); Task UpdateUnrestrictedLink(Guid downloadId, String unrestrictedLink); - Task UpdatePath(Guid downloadId, String path); + Task UpdatePath(Guid downloadId, String path); Task UpdateFileName(Guid downloadId, String fileName); Task UpdateDownloadStarted(Guid downloadId, DateTimeOffset? dateTime); Task UpdateDownloadFinished(Guid downloadId, DateTimeOffset? dateTime); diff --git a/server/RdtClient.Service/Services/TorrentRunner.cs b/server/RdtClient.Service/Services/TorrentRunner.cs index f00bba3d..4668bf4c 100644 --- a/server/RdtClient.Service/Services/TorrentRunner.cs +++ b/server/RdtClient.Service/Services/TorrentRunner.cs @@ -532,23 +532,30 @@ await remoteService.UpdateRateLimitStatus(new() { logger.LogError(ex, "Cannot unrestrict link: {ex.Message}", ex.Message); - if (download.RetryCount < torrent.DownloadRetryAttempts) + try { - var retryCount = download.RetryCount + 1; - var retryDelay = GetDownloadLinkRetryDelay(retryCount); - var retryAt = DateTimeOffset.UtcNow.Add(retryDelay); + if (download.RetryCount < torrent.DownloadRetryAttempts) + { + var retryCount = download.RetryCount + 1; + var retryDelay = GetDownloadLinkRetryDelay(retryCount); + var retryAt = DateTimeOffset.UtcNow.Add(retryDelay); - Log($"Retrying download link generation {retryCount}/{torrent.DownloadRetryAttempts} at {retryAt:u}", download, torrent); + Log($"Retrying download link generation {retryCount}/{torrent.DownloadRetryAttempts} at {retryAt:u}", download, torrent); - await downloads.Reset(download.DownloadId, retryAt); - await downloads.UpdateRetryCount(download.DownloadId, retryCount); + await downloads.Reset(download.DownloadId, retryAt); + await downloads.UpdateRetryCount(download.DownloadId, retryCount); + } + else + { + await downloads.UpdateError(download.DownloadId, ex.Message); + await downloads.UpdateCompleted(download.DownloadId, DateTimeOffset.UtcNow); + download.Error = ex.Message; + download.Completed = DateTimeOffset.UtcNow; + } } - else + catch (Exception updateEx) { - await downloads.UpdateError(download.DownloadId, ex.Message); - await downloads.UpdateCompleted(download.DownloadId, DateTimeOffset.UtcNow); - download.Error = ex.Message; - download.Completed = DateTimeOffset.UtcNow; + logger.LogError(updateEx, "Could not record the unrestrict failure for download: {updateEx.Message} {downloadInfo}", updateEx.Message, download.ToLog()); } return; @@ -773,7 +780,15 @@ await remoteService.UpdateRateLimitStatus(new() catch (Exception ex) { logger.LogError(ex.Message, "Torrent processing result in an unexpected exception: {Message}", ex.Message); - await torrents.UpdateComplete(torrent.TorrentId, $"Runner error: {ex.Message}", DateTimeOffset.UtcNow, true); + + try + { + await torrents.UpdateComplete(torrent.TorrentId, $"Runner error: {ex.Message}", DateTimeOffset.UtcNow, true); + } + catch (Exception updateEx) + { + logger.LogError(updateEx, "Could not mark torrent as errored: {updateEx.Message} {torrentInfo}", updateEx.Message, torrent.ToLog()); + } } } diff --git a/server/RdtClient.Service/Services/Torrents.cs b/server/RdtClient.Service/Services/Torrents.cs index a94748e6..cf28bee3 100644 --- a/server/RdtClient.Service/Services/Torrents.cs +++ b/server/RdtClient.Service/Services/Torrents.cs @@ -634,8 +634,12 @@ public async Task UnrestrictLink(Guid downloadId) if (refreshedPath != null) { - await downloads.UpdatePath(downloadId, refreshedPath); - download.Path = refreshedPath; + var updated = await downloads.UpdatePath(downloadId, refreshedPath); + + if (updated) + { + download.Path = refreshedPath; + } } }