release: CHDSharp 1.6.2 - extraction diagnostics

CHDSharpCli/README.md

@@ -17,7 +17,7 @@
 
 The binary is named `CHDSharp` (e.g. `CHDSharp.exe` on Windows). It accepts the same subcommands and options as MAME's `chdman`, plus additional convenience commands.
 
-> **v1.6.1** - expected user errors (bad flags, missing paths, sector-misaligned input, an existing output without `--force`) are logged locally but no longer posted to the Bug Report API; real failures still are. Six platform bundles are attached to every release (`CHDSharp_<os>-<arch>_v<version>.zip`). Release bundles are framework-dependent single-file binaries and require the .NET 10 runtime; the library targets `net8.0` / `net9.0` / `net10.0`.
+> **v1.6.2** - extraction failures keep their real cause: the underlying exception is logged, data/codec errors map to `Chderrdecompressionerror` and output I/O to `Chderrwriteerror`. Read-ahead teardown can no longer race a disposed semaphore, and damaged GD-ROM metadata returns `Chderrinvaliddata`. Six platform bundles are attached to every release (`CHDSharp_<os>-<arch>_v<version>.zip`). Release bundles are framework-dependent single-file binaries and require the .NET 10 runtime; the library targets `net8.0` / `net9.0` / `net10.0`.
 
 ---
 

CHDSharpLib/CHDFile.cs

@@ -3986,9 +3986,12 @@ public ChdError ExtractToc(string tocPath, string binPath, IProgress<ChdProgress
         {
             throw;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            return ChdError.Chderrwriteerror;
+            Log.LogWarning(ex, "Failed to write extracted data");
+            return ex is InvalidDataException
+                ? ChdError.Chderrdecompressionerror
+                : ChdError.Chderrwriteerror;
         }
     }
 
@@ -4138,6 +4141,11 @@ public ExtractResult ExtractToDirectoryWithReporting(
 
         if (IsGdRom)
         {
+            // Damaged CD metadata can identify a GD-ROM without yielding any tracks; report
+            // invalid data instead of dereferencing a null track list.
+            if (Tracks is not { Count: > 0 })
+                return new ExtractResult(created, trackResults, ChdError.Chderrinvaliddata);
+
             var gdTrackFiles = new List<string>();
             foreach (var track in Tracks!)
             {
@@ -4169,8 +4177,14 @@ public ExtractResult ExtractToDirectoryWithReporting(
                 File.WriteAllText(gdiFile, GenerateGdiDescriptor(trackNames, cooked));
                 created.Add(gdiFile);
             }
-            catch (Exception)
+            catch (Exception ex)
             {
+                Log.LogWarning(
+                    ex,
+                    "Failed to write the GDI descriptor for '{BaseFileName}' in '{OutputDir}'",
+                    baseFileName,
+                    outputDir
+                );
                 return new ExtractResult(created, trackResults, ChdError.Chderrwriteerror);
             }
 
@@ -4228,6 +4242,7 @@ public ExtractResult ExtractToDirectoryWithReporting(
         }
         catch (Exception ex)
         {
+            Log.LogWarning(ex, "Failed to write extracted data");
             return new ExtractResult(
                 created,
                 trackResults,
@@ -4369,9 +4384,12 @@ CancellationToken cancellationToken
         {
             throw;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            return ChdError.Chderrwriteerror;
+            Log.LogWarning(ex, "Failed to write extracted data");
+            return ex is InvalidDataException
+                ? ChdError.Chderrdecompressionerror
+                : ChdError.Chderrwriteerror;
         }
     }
 
@@ -4473,9 +4491,12 @@ CancellationToken cancellationToken
         {
             throw;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            return ChdError.Chderrwriteerror;
+            Log.LogWarning(ex, "Failed to write extracted data");
+            return ex is InvalidDataException
+                ? ChdError.Chderrdecompressionerror
+                : ChdError.Chderrwriteerror;
         }
     }
 
@@ -4602,9 +4623,12 @@ CancellationToken cancellationToken
         {
             throw;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            return ChdError.Chderrwriteerror;
+            Log.LogWarning(ex, "Failed to write extracted data");
+            return ex is InvalidDataException
+                ? ChdError.Chderrdecompressionerror
+                : ChdError.Chderrwriteerror;
         }
     }
 
@@ -4772,9 +4796,12 @@ CancellationToken cancellationToken
         {
             throw;
         }
-        catch (Exception)
+        catch (Exception ex)
         {
-            return ChdError.Chderrwriteerror;
+            Log.LogWarning(ex, "Failed to write extracted data");
+            return ex is InvalidDataException
+                ? ChdError.Chderrdecompressionerror
+                : ChdError.Chderrwriteerror;
         }
     }
 
@@ -4848,7 +4875,19 @@ internal bool TryGet(uint hunknum, byte[] buffer)
                 return false;
 
             Array.Copy(data, 0, buffer, 0, _chd._chd.Blocksize);
-            _semaphore.Release();
+
+            // Disposal may release the semaphore while a cached hunk is still being consumed.
+            // The permit is irrelevant once the manager is gone, so a disposed semaphore is not
+            // an error and must never surface as an unobserved exception.
+            try
+            {
+                _semaphore.Release();
+            }
+            catch (ObjectDisposedException)
+            {
+                // The manager was disposed after the cache entry was removed.
+            }
+
             return true;
         }
 
@@ -4952,8 +4991,17 @@ internal void Clear()
             // Each cached entry owns a semaphore permit; release it when discarding.
             foreach (var key in _cache.Keys)
             {
-                if (_cache.TryRemove(key, out _))
+                if (!_cache.TryRemove(key, out _))
+                    continue;
+
+                try
+                {
                     _semaphore.Release();
+                }
+                catch (ObjectDisposedException)
+                {
+                    // The manager was disposed while the cache was being cleared.
+                }
             }
         }
 

CHDSharpLib/CHDSharpLib.csproj

@@ -18,7 +18,7 @@
         <RepositoryType>git</RepositoryType>
         <PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
         <PackageLicenseExpression></PackageLicenseExpression>
-        <PackageReleaseNotes>v1.6.1: bug-fix release. Expected user/usage errors in the CLI (bad flags, missing paths, sector-misaligned input, an existing output without --force, overridden format options) are now logged through ILogger.UserError and tagged so BugReportSink never posts them to the Bug Report API; genuine runtime failures are still reported. Adds a DocFX-generated per-member API reference published with the docs site (0 metadata warnings) and moves Pages deployment to a dedicated workflow. Console text and exit codes are unchanged (chdman parity). No public API changes. Pure C# - zero native dependencies. Targets net8.0/net9.0/net10.0.</PackageReleaseNotes>
+        <PackageReleaseNotes>v1.6.2: extraction diagnostics release. File-writing failures during extraction now log the real exception and return Chderrdecompressionerror for data/codec failures or Chderrwriteerror for output I/O instead of flattening every failure to Chderrwriteerror. Read-ahead teardown no longer races a disposed semaphore, and damaged GD-ROM metadata returns Chderrinvaliddata instead of throwing. No encoder output changes: the quick battle harness stays 3,174/3,174 checks byte-identical to chdman 0.289, and CD/GD-ROM extraction is verified byte-identical to chdman extractcd. No public API changes. Pure C# - zero native dependencies. Targets net8.0/net9.0/net10.0.</PackageReleaseNotes>
         <GenerateDocumentationFile>true</GenerateDocumentationFile>
         <WarningsAsErrors>$(WarningsAsErrors);CS1591</WarningsAsErrors>
     </PropertyGroup>

CHDSharpLib/README.md

@@ -20,6 +20,12 @@
 
 ---
 
+## What's New in v1.6.2
+
+- **Extraction failures keep their real cause** - a failed extracted-file write now logs the underlying exception and returns `Chderrdecompressionerror` for data/codec failures or `Chderrwriteerror` for output I/O instead of reporting every exception as a write error.
+- **Read-ahead teardown race fixed** - `ReadAheadManager.TryGet`/`Clear` ignore a semaphore released after disposal, so teardown cannot surface an unobserved `ObjectDisposedException`.
+- **Damaged GD-ROM metadata is reported** - `ExtractToDirectoryWithReporting` returns `Chderrinvaliddata` when a GD-ROM CHD has no readable tracks instead of dereferencing a null track list.
+- **No public API changes** - 4,219 library tests + 455 encoder tests, `jb inspectcode` 0 findings, quick battle harness 3,174/3,174 checks with byte-identical `chdman` output.
 ## What's New in v1.6.1
 
 - **Automatic bug reports now only cover real bugs** - CLI diagnostics for expected user input (bad flags, invalid numbers, missing paths, sector-misaligned sources, an existing output without `--force`, overridden format options) are logged at warning level but tagged so the Bug Report API never receives them; runtime failures are still reported. No library code changed.

CHDSharpTest/ExtractionFailureLoggingTests.cs

@@ -0,0 +1,106 @@
+using Microsoft.Extensions.Logging;
+
+namespace CHDSharp.Tests;
+
+public sealed class ExtractionFailureLoggingTests
+{
+    private static readonly string TestDataDir = Path.Combine(AppContext.BaseDirectory, "TestData");
+
+    [Fact]
+    public void ExtractToDirectoryWithReporting_UnwritableTarget_LogsRealException()
+    {
+        var outputDir = Path.Combine(Path.GetTempPath(), $"chd_extract_fail_{Guid.NewGuid():N}");
+        var previous = Chd.LoggerFactory;
+        var capture = new CaptureLoggerFactory();
+        Chd.LoggerFactory = capture;
+        try
+        {
+            Directory.CreateDirectory(outputDir);
+            // A directory where the output BIN must go makes FileStream creation fail.
+            Directory.CreateDirectory(Path.Combine(outputDir, "test.bin"));
+
+            var err = ChdFile.Open(Path.Combine(TestDataDir, "v5_cd_default.chd"), out var chd);
+            Assert.Equal(ChdError.Chderrnone, err);
+            using (chd)
+            {
+                var result = chd!.ExtractToDirectoryWithReporting(
+                    outputDir,
+                    "test",
+                    cooked: true
+                );
+                Assert.Equal(ChdError.Chderrwriteerror, result.Error);
+            }
+
+            lock (capture.Entries)
+            {
+                Assert.Contains(
+                    capture.Entries,
+                    e =>
+                        e.Level == LogLevel.Warning
+                        && e.Message.Contains(
+                            "Failed to write extracted data",
+                            StringComparison.Ordinal
+                        )
+                        && e.Exception is not null
+                );
+            }
+        }
+        finally
+        {
+            Chd.LoggerFactory = previous;
+            try
+            {
+                if (Directory.Exists(outputDir))
+                    Directory.Delete(outputDir, true);
+            }
+            catch
+            {
+                // ignored
+            }
+        }
+    }
+
+    private sealed class CaptureLoggerFactory : ILoggerFactory
+    {
+        public List<(LogLevel Level, string Message, Exception? Exception)> Entries { get; } = [];
+
+        public void AddProvider(ILoggerProvider provider)
+        {
+            ArgumentNullException.ThrowIfNull(provider);
+        }
+
+        public ILogger CreateLogger(string categoryName)
+        {
+            ArgumentNullException.ThrowIfNull(categoryName);
+            return new CaptureLogger(this);
+        }
+
+        public void Dispose() { }
+
+        private sealed class CaptureLogger(CaptureLoggerFactory owner) : ILogger
+        {
+            public IDisposable? BeginScope<TState>(TState state)
+                where TState : notnull
+            {
+                return null;
+            }
+
+            public bool IsEnabled(LogLevel logLevel)
+            {
+                return true;
+            }
+
+            public void Log<TState>(
+                LogLevel logLevel,
+                EventId eventId,
+                TState state,
+                Exception? exception,
+                Func<TState, Exception?, string> formatter
+            )
+            {
+                lock (owner.Entries)
+                    owner.Entries.Add((logLevel, formatter(state, exception), exception));
+            }
+        }
+    }
+}

CHDSharpTest/README.md

@@ -14,7 +14,7 @@
 
 **xUnit unit and corpus test suite for CHDSharp — tests header parsing, CRC checksums, and full decompression against a deterministic 30-file CHD corpus (V1–V5, all codecs).**
 
-> **v1.6.1** - targets `net8.0` / `net9.0` / `net10.0`. 4,216 tests per target framework; encoder suite 455 tests; full battle harness 3,174/3,174 checks vs `chdman` 0.289; solution clean under `jb inspectcode`. v1.6.1 adds CLI regression tests for the `createdvd` validation paths.
+> **v1.6.2** - targets `net8.0` / `net9.0` / `net10.0`. 4,219 tests per target framework; encoder suite 455 tests; full battle harness 3,174/3,174 checks vs `chdman` 0.289; solution clean under `jb inspectcode`. v1.6.2 adds extraction-failure logging and read-ahead teardown-race regression tests.
 
 ---
 

CHDSharpTestDataGeneration/README.md

@@ -14,7 +14,7 @@
 
 **Deterministic CHD test corpus generator for the CHDSharp xUnit test suite.**
 
-> **v1.6.1** - targets `net8.0` / `net9.0` / `net10.0`. Generates the 30-file corpus for V1-V5 via MAME 0.289 `chdman`; version-aligned with the v1.6.1 release.
+> **v1.6.2** - targets `net8.0` / `net9.0` / `net10.0`. Generates the 30-file corpus for V1-V5 via MAME 0.289 `chdman`; version-aligned with the v1.6.2 release.
 
 Generates the 30-file CHD corpus in `CHDSharpTest/TestData/`, covering every format version (V1–V5), all compression codecs (zlib, lzma, huffman, flac, zstd, avhu + CD variants), every map type, and parent/child chains. All source images are built deterministically (seeded PRNG) so regenerating the corpus produces byte-identical output.
 

Directory.Build.props

@@ -1,8 +1,8 @@
 <Project>
   <PropertyGroup>
-    <Version>1.6.1</Version>
-    <AssemblyVersion>1.6.1</AssemblyVersion>
-    <FileVersion>1.6.1</FileVersion>
+    <Version>1.6.2</Version>
+    <AssemblyVersion>1.6.2</AssemblyVersion>
+    <FileVersion>1.6.2</FileVersion>
     <Nullable>enable</Nullable>
     <ImplicitUsings>enable</ImplicitUsings>
     <DebugType>embedded</DebugType>

README.md

@@ -26,6 +26,13 @@ Supports every CHD format version (V1–V5), all 10 compression codecs, parent/c
 
 ---
 
+## What's New in v1.6.2
+
+- **Extraction failures keep their real cause** - the extraction writers no longer flatten every exception to `Chderrwriteerror`. The underlying exception is logged at warning level, data/codec failures return `Chderrdecompressionerror`, and genuine output I/O failures return `Chderrwriteerror`. This is the fix behind the opaque `track N: Chderrwriteerror` reports.
+- **Read-ahead teardown no longer races a cached hunk** - `ReadAheadManager.TryGet` and `Clear` tolerate a semaphore disposed between cache removal and permit release, so teardown cannot throw `ObjectDisposedException`.
+- **Damaged GD-ROM metadata returns an error instead of crashing** - `ExtractToDirectoryWithReporting` returns `Chderrinvaliddata` when a GD-ROM CHD yields no tracks.
+- **No encoder output changes** - quick battle harness 3,174/3,174 checks, byte-identical to `chdman` 0.289; CD (V3/V4/V5, cdfl/cdlz/cdzl/cdzs/default) and synthetic GD-ROM GDI extraction verified byte-identical to `chdman extractcd`.
+- **Verification** - 4,219 library tests + 455 encoder tests, `jb inspectcode` 0 findings.
 ## What's New in v1.6.1
 
 - **The bug-report API only receives real bugs** - expected user/usage errors are tagged via `logger.UserError(...)` and stay in the console and file logs without being posted as defects. That covers bad flags, invalid numbers, missing paths, sector-misaligned input, an existing output without `--force`, overridden format options and unsupported combinations (216 call sites). Genuine failures (`Open failed`, `* failed: {Message}`, verification and extraction failures) are still reported.