From eda922c3f305b6084de9a4ea5b31f7f27213d2a2 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Sat, 13 Dec 2025 11:55:58 +1030 Subject: [PATCH 1/2] Implement DisposeAsync Log when snapshot is added --- .../Services/ROMArchiveReader.cs | 30 ++++++++-- TinfoilVibeServer/Services/SnapshotService.cs | 58 ++++++++++--------- 2 files changed, 56 insertions(+), 32 deletions(-) diff --git a/TinfoilVibeServer/Services/ROMArchiveReader.cs b/TinfoilVibeServer/Services/ROMArchiveReader.cs index cecad01..f5ef348 100644 --- a/TinfoilVibeServer/Services/ROMArchiveReader.cs +++ b/TinfoilVibeServer/Services/ROMArchiveReader.cs @@ -17,7 +17,7 @@ namespace TinfoilVibeServer.Services /// /// Reads a ROM archive (zip / 7z / rar) from a stream. /// - public sealed class RomArchiveReader : IDisposable + public sealed class RomArchiveReader : IDisposable, IAsyncDisposable { private readonly ZipArchive? _zipArchive; private readonly IArchive? _archive; @@ -165,9 +165,31 @@ namespace TinfoilVibeServer.Services /// public void Dispose() { - _zipArchive?.Dispose(); - _archive?.Dispose(); - _archiveStream?.Dispose(); + DisposeAsync().GetAwaiter().GetResult(); + } + + public async ValueTask DisposeAsync() + { + // Dispose ZipArchive (no async support – just dispose synchronously) + if (_zipArchive is IAsyncDisposable asyncZip) + await asyncZip.DisposeAsync().ConfigureAwait(false); + else + _zipArchive?.Dispose(); + // Dispose SharpCompress IArchive (no async support) + if (_archive is IAsyncDisposable asyncArc) + await asyncArc.DisposeAsync().ConfigureAwait(false); + else + _archive?.Dispose(); + // Dispose the underlying stream (may support async) + if (_archiveStream is IAsyncDisposable asyncStream) + await asyncStream.DisposeAsync().ConfigureAwait(false); + else + _archiveStream?.Dispose(); + // Avoid double‑dispose if the caller disposes again. + // (not strictly required but prevents accidental misuse) + // _zipArchive = null; // not possible – fields are readonly + // _archive = null; + // _archiveStream = null; } /// diff --git a/TinfoilVibeServer/Services/SnapshotService.cs b/TinfoilVibeServer/Services/SnapshotService.cs index 281993e..161c1ef 100644 --- a/TinfoilVibeServer/Services/SnapshotService.cs +++ b/TinfoilVibeServer/Services/SnapshotService.cs @@ -24,7 +24,7 @@ public interface ISnapshotService Task RebuildSnapshotAsync(CancellationToken cancellationToken = default); SnapshotService.ROMSnapshot GetSnapshot(); - Task AddToSnapshotAsync(FileEntry entry); + Task AddToSnapshotAsync(FileEntry entry, CancellationToken cancellationToken = default); Task BuildSnapshotAsync(CancellationToken cancellationToken = default); void GetArchiveName(string titleId); char GetArchivePathSeparator(); @@ -308,13 +308,13 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ #region Snapshot logic - public Task AddToSnapshotAsync(FileEntry entry) + public async Task AddToSnapshotAsync(FileEntry entry, CancellationToken cancellationToken = default) { // Update lookup tables if (entry.Hash == null) { _logger.LogWarning("Cannot add entry {Path} to snapshot: no hash", entry.Path); - return Task.CompletedTask; + return; } var lastModified = File.GetLastWriteTimeUtc(entry.Path.Contains(ArchivePathSeparator) ? entry.Path.Split(ArchivePathSeparator)[0] : entry.Path); @@ -343,8 +343,9 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ } // Persist snapshot to disk - PersistSnapshotAsync(); - return Task.CompletedTask; + var titleIds = entry.Titles.Aggregate("", (current, titles) => $"{current} ,{titles.TitleId}"); + _logger.LogInformation("Added {Path} to snapshot, titleIds=[{TitleIds}]", entry.Path, titleIds); + await PersistSnapshotAsync(cancellationToken); } /* ============================================================== @@ -452,7 +453,8 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ { //var titleInfo = _titleDatabaseService.GetAsync(ncaMetadataWithHash.TitleId).Result; var fileEntryFromFileName = new FileEntry(file, fileInfo.Length, ncaMetadataWithHash.Hash, [ncaMetadataWithHash]); - AddToSnapshotAsync(fileEntryFromFileName); + var addToSnapshotAsync = AddToSnapshotAsync(fileEntryFromFileName, cancellationToken); + addToSnapshotAsync.Wait(cancellationToken); cancellationToken.ThrowIfCancellationRequested(); yield return fileEntryFromFileName; continue; @@ -472,7 +474,8 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ if (title != null) { var romEntry = new FileEntry(file, nspStreamLength, hash, [title]); - AddToSnapshotAsync(romEntry); + var addToSnapshotAsync = AddToSnapshotAsync(romEntry, cancellationToken); + addToSnapshotAsync.Wait(cancellationToken); titles.Add((title.TitleId, nspStreamLength, title)); cancellationToken.ThrowIfCancellationRequested(); yield return romEntry; @@ -486,7 +489,7 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ if (_archiveLookup.ContainsKey(file)) continue; if (processedFiles.Contains(file)) continue; _logger.LogDebug("Extracting hash for {File}", file); - Stopwatch stopwatch = Stopwatch.StartNew(); + var stopwatch = Stopwatch.StartNew(); hash = ComputeFirstStreamHashAsync(file, cancellationToken).Result; stopwatch.Stop(); if (!string.IsNullOrEmpty(hash)) @@ -527,7 +530,8 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ foreach (var title in titles) { var archiveEntry = new FileEntry(file + ArchivePathSeparator + title.Item1, title.Item2, title.Item3.Hash, [title.Item3]); - AddToSnapshotAsync(archiveEntry); + var addToSnapshotAsync = AddToSnapshotAsync(archiveEntry, cancellationToken); + addToSnapshotAsync.Wait(cancellationToken); cancellationToken.ThrowIfCancellationRequested(); yield return archiveEntry; } @@ -564,11 +568,15 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ } catch (IOException ex) when (attempt < _options.MaxRetryCount - 1) { - var delay = (int)((attempt+1) * _options.DebounceTimeoutMs * _options.RetryMultiplier); - _logger.LogWarning(ex, "Attempt {Attempt} failed for {Path}. Retrying after {Delay}.", - attempt + 1, file, delay); + var delay = (int)((attempt + 1) * _options.DebounceTimeoutMs * _options.RetryMultiplier); + _logger.LogWarning(ex, "Failed to load {Path}. Attempt {Attempt}, Retrying after {Delay}.", + file, attempt + 1, delay); await Task.Delay(delay, cancellationToken); } + catch (IOException) when (attempt >= _options.MaxRetryCount - 1) + { + _logger.LogWarning("Load {Path} failed after {retries} attempts", file, attempt + 1); + } } return null; } @@ -859,7 +867,7 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ // The first stream is the first entry returned by GetContentInfos(). try { - using var reader = new RomArchiveReader(filePath); + await using var reader = new RomArchiveReader(filePath); var first = reader.GetEntries().FirstOrDefault(); if (first == null) return ComputeFullHash(filePath); @@ -871,35 +879,29 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ } catch { - // On error, fall back to the full file hash - await using var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read); - var ncaMetadataWithHash = _nspExtractor.ExtractFromStream(fs); - return ncaMetadataWithHash?.Hash ?? string.Empty; + // ignored } } - else - { - await using var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read); - var ncaMetadataWithHash = _nspExtractor.ExtractFromStream(fs); - return ncaMetadataWithHash?.Hash ?? string.Empty; - } + + await using var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.Read); + var ncaMetadataWithHash = _nspExtractor.ExtractFromStream(fs); + return ncaMetadataWithHash?.Hash ?? string.Empty; } catch (IOException ex) when (attempt < _options.MaxRetryCount - 1) { var delay = (int)((attempt + 1) * _options.DebounceTimeoutMs * _options.RetryMultiplier); - _logger.LogWarning(ex, "Attempt {Attempt} failed for {Path}. Retrying after {Delay}.", - attempt + 1, filePath, delay); + _logger.LogWarning(ex, "Failed to load {Path}. Attempt {Attempt}, Retrying after {Delay}.", + filePath, attempt + 1, delay); await Task.Delay(delay, cancellationToken); } - catch (IOException) + catch (IOException) when (attempt >= _options.MaxRetryCount - 1) { - _logger.LogWarning("Attempt to load {Path} failed after {retries}", filePath, attempt + 1); + _logger.LogWarning("Load {Path} failed after {retries} attempts", filePath, attempt + 1); return null; } } return string.Empty; - throw new IOException($"Failed to compute hash for {filePath} after {_options.MaxRetryCount} attempts"); } private static string ComputeFullHash(string filePath) -- 2.52.0 From f710a4cbde42654804de3d82c6cd89bc195647bc Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Sun, 14 Dec 2025 10:05:53 +1030 Subject: [PATCH 2/2] Check lock against base archive in multipart scenario Explicitly attempt to close Reader when using ArchiveFactory --- .../Services/ROMArchiveReader.cs | 47 ++++++++--------- TinfoilVibeServer/Services/SnapshotService.cs | 50 +++++++++++-------- 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/TinfoilVibeServer/Services/ROMArchiveReader.cs b/TinfoilVibeServer/Services/ROMArchiveReader.cs index f5ef348..975b2b6 100644 --- a/TinfoilVibeServer/Services/ROMArchiveReader.cs +++ b/TinfoilVibeServer/Services/ROMArchiveReader.cs @@ -22,7 +22,6 @@ namespace TinfoilVibeServer.Services private readonly ZipArchive? _zipArchive; private readonly IArchive? _archive; private readonly Stream? _archiveStream; // the stream actually handed to SharpCompress - private readonly ICollection? _partStreams; public RomArchiveReader(string path) { @@ -55,7 +54,15 @@ namespace TinfoilVibeServer.Services stream.CopyTo(ms); ms.Position = 0; _archiveStream = ms; - stream.Dispose(); // original non‑seekable stream no longer needed + if (stream is IAsyncDisposable asyncDisposable) + { + var disposeAsync = asyncDisposable.DisposeAsync(); + disposeAsync.ConfigureAwait(false); + } + else + { + stream.Dispose(); // original non‑seekable stream no longer needed + } } else { @@ -73,28 +80,29 @@ namespace TinfoilVibeServer.Services // Detect whether the file is a multi‑part RAR and wrap it if necessary private static IArchive DetectAndWrap(string path) { - string ext = Path.GetExtension(path).ToLowerInvariant(); + var ext = Path.GetExtension(path).ToLowerInvariant(); - if (ext == ".rar" || ext == ".r00" || ext == ".r01" || ext == ".r02") + if (ext is ".rar" or ".r00" or ".r01" or ".r02") { - var dir = Path.GetDirectoryName(path)!; + var dir = Path.GetDirectoryName(path)!; var fileName = Path.GetFileName(path); // ----- 1️⃣ Determine the base name (everything before the first ".rar" or ".partNN") ----- - string baseName = MultiPartRarHelper.GetBaseNameForRarVolume(fileName); + var baseName = MultiPartRarHelper.GetBaseNameForRarVolume(fileName); // Any file that ends with .rar or .rNN could be the start of a multi‑part set // Let MultiPartRarStream decide which parts belong together. var volumes = MultiPartRarHelper.DiscoverVolumes(dir, baseName); if (volumes.Count is 0 or 1) { - return ArchiveFactory.Open(path); + return ArchiveFactory.Open(path, new ReaderOptions { LeaveStreamOpen = false }); } var streams = new List(volumes.Count); foreach (var volume in volumes) { - streams.Add(new FileStream(volume, FileMode.Open, FileAccess.Read, FileShare.Read)); + // todo: check all streams for read validity? The rar may be available but the parts are not all downloaded yet + streams.Add(new FileStream(volume, FileMode.Open, FileAccess.Read, FileShare.Read, 10*1024*1024, FileOptions.Asynchronous)); } return ArchiveFactory.Open(streams, new ReaderOptions { LeaveStreamOpen = false }); @@ -102,7 +110,7 @@ namespace TinfoilVibeServer.Services // Normal single‑file archive (zip, 7z, single‑rar, etc.) using var archiveStream = File.OpenRead(path); - return ArchiveFactory.Open(archiveStream); + return ArchiveFactory.Open(archiveStream, new ReaderOptions { LeaveStreamOpen = false }); } private static Stream? GetPart(int arg) @@ -170,26 +178,15 @@ namespace TinfoilVibeServer.Services public async ValueTask DisposeAsync() { - // Dispose ZipArchive (no async support – just dispose synchronously) - if (_zipArchive is IAsyncDisposable asyncZip) - await asyncZip.DisposeAsync().ConfigureAwait(false); - else - _zipArchive?.Dispose(); - // Dispose SharpCompress IArchive (no async support) - if (_archive is IAsyncDisposable asyncArc) - await asyncArc.DisposeAsync().ConfigureAwait(false); - else - _archive?.Dispose(); - // Dispose the underlying stream (may support async) + _zipArchive?.Dispose(); + _archive?.Dispose(); + + // Dispose of the underlying stream (may support async) if (_archiveStream is IAsyncDisposable asyncStream) await asyncStream.DisposeAsync().ConfigureAwait(false); else + // ReSharper disable once MethodHasAsyncOverload _archiveStream?.Dispose(); - // Avoid double‑dispose if the caller disposes again. - // (not strictly required but prevents accidental misuse) - // _zipArchive = null; // not possible – fields are readonly - // _archive = null; - // _archiveStream = null; } /// diff --git a/TinfoilVibeServer/Services/SnapshotService.cs b/TinfoilVibeServer/Services/SnapshotService.cs index 161c1ef..7c246a8 100644 --- a/TinfoilVibeServer/Services/SnapshotService.cs +++ b/TinfoilVibeServer/Services/SnapshotService.cs @@ -319,6 +319,7 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ var lastModified = File.GetLastWriteTimeUtc(entry.Path.Contains(ArchivePathSeparator) ? entry.Path.Split(ArchivePathSeparator)[0] : entry.Path); + var cacheUpdated = _cache.ContainsKey(entry.Path); _cache[entry.Path] = new SnapshotEntry(entry.Path, entry.Hash, entry.Size, lastModified, entry.Titles); _hashCache[entry.Hash] = entry.Path; _sizeLookup[entry.Hash] = entry.Size; @@ -339,12 +340,13 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ _hashCache[ncaMetadataWithHash.Hash] = entry.Path; _sizeLookup[ncaMetadataWithHash.Hash] = entry.Size; - //_logger.LogInformation("Added entry {titleId} to snapshot (hash={hash})", ncaMetadataWithHash.TitleId, ncaMetadataWithHash.Hash); } // Persist snapshot to disk - var titleIds = entry.Titles.Aggregate("", (current, titles) => $"{current} ,{titles.TitleId}"); - _logger.LogInformation("Added {Path} to snapshot, titleIds=[{TitleIds}]", entry.Path, titleIds); + // If entry.Titles is null, treat it as an empty collection + var titleIds = string.Join(",", entry.Titles.Select(t => t.TitleId.ToString())); + _logger.LogInformation(cacheUpdated ? "Updated snapshot for {Path}, titleIds=[{TitleIds}]" : "Added {Path} to snapshot, titleIds=[{TitleIds}]", entry.Path, titleIds); + await PersistSnapshotAsync(cancellationToken); } @@ -406,7 +408,7 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ } } - var currentHash = ComputeSnapshotHash(entries); + //var currentHash = ComputeSnapshotHash(entries); if (entries.Count > 0 || fileInfo.Exists && index.Count == 0) SnapshotRebuilt?.Invoke(this, EventArgs.Empty); } @@ -596,6 +598,8 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ private Task PersistSnapshotAsync(CancellationToken cancellationToken = default) { + cancellationToken.ThrowIfCancellationRequested(); + if (_debouncerCache.TryGetValue(_jsonPath, out _)) { _logger.LogDebug("Sliding debounce in progress, skipping snapshot persistence"); @@ -623,26 +627,25 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ if (reason is not (EvictionReason.Expired or EvictionReason.TokenExpired)) return; var filePath = (string)key; - if (_snapshotFileSemaphore.Wait(SnapshotFileLockTimeout)) + if (!_snapshotFileSemaphore.Wait(SnapshotFileLockTimeout)) return; + + try { - try + if (FileLockHelper.IsFileLocked(filePath)) { - if (FileLockHelper.IsFileLocked(filePath)) - { - _logger.LogInformation("File {FilePath} is locked, skipping snapshot persistence", filePath); - } - else - { - File.WriteAllText(filePath, JsonSerializer.Serialize(value, _jsonSerializerOptions)); - _logger.LogInformation("Persisted snapshot to {FilePath}", filePath); - SnapshotRebuilt?.Invoke(this, EventArgs.Empty); - } + _logger.LogInformation("File {FilePath} is locked, skipping snapshot persistence", filePath); } - finally + else { - _snapshotFileSemaphore.Release(); + File.WriteAllText(filePath, JsonSerializer.Serialize(value, _jsonSerializerOptions)); + _logger.LogInformation("Persisted snapshot to {FilePath}", filePath); + SnapshotRebuilt?.Invoke(this, EventArgs.Empty); } } + finally + { + _snapshotFileSemaphore.Release(); + } } } } @@ -854,13 +857,19 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ { try { + var ext = Path.GetExtension(filePath).ToLowerInvariant(); + + var multiPartBasePathWithExtension = $"{MultiPartRarHelper.GetBaseNameForRarVolume(filePath)}{ext}"; + if (string.CompareOrdinal(multiPartBasePathWithExtension, filePath) != 0) + { + filePath = multiPartBasePathWithExtension; + } if (FileLockHelper.IsFileLocked(filePath)) { throw new IOException("File is locked"); } // Only treat NSP/XCI/XCZ as “first‑stream” files - var ext = Path.GetExtension(filePath).ToLowerInvariant(); if (ext is not ".nsp" and not ".xci" and not ".xcz") { // Open the NSP/XCI with LibHac and read the first stream. @@ -868,12 +877,11 @@ public sealed class SnapshotService : IDisposable, ISnapshotService, IHostedServ try { await using var reader = new RomArchiveReader(filePath); - var first = reader.GetEntries().FirstOrDefault(); if (first == null) return ComputeFullHash(filePath); //using var seekableWrapper = new SeekableBufferedStream(first.Stream, first.Stream.Length, 10*1024*1024, true); - await using var rewindableWrapper = new RewindableStream(first.Stream, () => { return reader.GetEntries().FirstOrDefault().Stream; }, 10 * 1024 * 1024, first.Stream.Length); + await using var rewindableWrapper = new RewindableStream(first.Stream, () => first.Stream, 10 * 1024 * 1024, first.Stream.Length); var hash = _nspExtractor.ExtractHashFromStream(rewindableWrapper); return hash; } -- 2.52.0