Fix sync methods called in async RAR unpacker paths (Unpack29Async, U…#1306
Merged
Conversation
…npack5Async) Agent-Logs-Url: https://github.com/adamhathcock/sharpcompress/sessions/7f2fcb59-4a41-4b27-ab32-17afec510b5e Co-authored-by: adamhathcock <527620+adamhathcock@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes async RAR extraction paths that were still invoking synchronous read/write code (breaking async-only streams), by introducing async equivalents in the RAR unpackers and PPMd glue code, plus adding regression tests to ensure async APIs don’t fall back to sync I/O.
Changes:
- Refactor RAR v1/v5 async unpacking to call newly added async table/VM/filter and buffer-flush methods instead of sync counterparts.
- Extend the IRarUnpack + PPMd integration to support async character reads and async PPM initialization/decoding.
- Add/expand async-only stream regression tests for RAR readers/archives; update ILLink.Tasks versions in the lock file.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/SharpCompress.Test/Rar/RarReaderAsyncTests.cs | Adds a theory to validate async RAR reader extraction works with an async-only stream wrapper. |
| tests/SharpCompress.Test/Rar/RarArchiveTests.cs | Adds a sync extraction smoke test over multiple RAR fixtures. |
| tests/SharpCompress.Test/Rar/RarArchiveAsyncTests.cs | Adds an async extraction smoke test over multiple RAR fixtures. |
| tests/SharpCompress.Test/Mocks/AsyncOnlyStream.cs | Refactors AsyncOnlyStream to use a primary constructor. |
| src/SharpCompress/packages.lock.json | Adjusts resolved/requested versions of Microsoft.NET.ILLink.Tasks for net10.0 and net8.0. |
| src/SharpCompress/IO/AsyncBinaryReader.cs | Minor refactor to expression-bodied async methods. |
| src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs | Replaces remaining sync buffer flush + “last tables” logic with async equivalents. |
| src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack15_async.cs | Switches remaining sync buffer flush calls to async. |
| src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.cs | Replaces Char property with ReadChar()/ReadCharAsync() for PPMd integration. |
| src/SharpCompress/Compressors/Rar/UnpackV1/Unpack50.Async.cs | Converts ReadTables / UnpWriteBuf / filter-add path to async in Unpack5Async. |
| src/SharpCompress/Compressors/Rar/UnpackV1/Unpack20.Async.cs | Switches final table read + buffer flush to async and adds async “last tables” helper. |
| src/SharpCompress/Compressors/Rar/UnpackV1/Unpack15.Async.cs | Switches remaining sync buffer flush calls to async. |
| src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs | Replaces Char property with ReadChar() for PPMd integration. |
| src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.Async.cs | Refactors Unpack29Async to fully async table/VM/PPM paths; adds async implementations of key helper methods. |
| src/SharpCompress/Compressors/Rar/IRarUnpack.cs | Updates the interface from Char to ReadChar() + ReadCharAsync(...). |
| src/SharpCompress/Compressors/PPMd/H/RangeCoder.cs | Switches to ReadChar()/ReadCharAsync() and adds async init from IRarUnpack. |
| src/SharpCompress/Compressors/PPMd/H/ModelPPM.cs | Switches sync init to ReadChar() and adds async init using IRarUnpack. |
| src/SharpCompress/Common/Rar/AsyncMarkingBinaryReader.cs | Wraps incomplete async reads as InvalidFormatException for consistency with sync reader behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+10
| public class AsyncOnlyStream(Stream stream) : Stream | ||
| { | ||
| private readonly Stream _stream; | ||
|
|
||
| public AsyncOnlyStream(Stream stream) | ||
| { | ||
| _stream = stream ?? throw new ArgumentNullException(nameof(stream)); | ||
| } | ||
| private readonly Stream _stream = stream ?? throw new ArgumentNullException(nameof(stream)); |
There was a problem hiding this comment.
AsyncOnlyStream is intended to enforce async-only usage, but it still allows synchronous operations like Write(...) (and Flush/WriteByte via the base implementation). This undermines tests that wrap output streams with AsyncOnlyStream to detect accidental sync writes. Consider throwing NotSupportedException for synchronous write/flush APIs as well (and optionally overriding WriteByte/Flush/FlushAsync as needed) so the helper reliably catches sync usage.
This was referenced May 6, 2026
This was referenced May 11, 2026
This was referenced May 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #1304
Supercedes: #1305
This pull request refactors and improves the asynchronous RAR unpacking logic in the
SharpCompresslibrary. The main focus is on converting several previously synchronous methods to asynchronous, ensuring that I/O operations do not block the calling thread. Additionally, there are minor dependency version adjustments in the project lock file.Asynchronous unpacking improvements:
Unpack29Asyncmethod inUnpack.Async.csto call new asynchronous versions ofReadTables,ReadEndOfBlock, andReadVMCode, replacing the previous synchronous calls. This ensures that all table reading and related operations are non-blocking. [1] [2] [3]ReadTablesAsync,ReadEndOfBlockAsync, andReadVMCodeAsyncinUnpack.Async.cs, implementing the logic of their synchronous counterparts usingawaitfor buffer reads and other async operations.Unpack5AsyncinUnpack50.Async.csto use the newReadTablesAsyncmethod instead of the synchronous version. [1] [2]Dependency version updates:
Microsoft.NET.ILLink.Taskspackage to version10.0.0fornet10.0and to8.0.22fornet8.0inpackages.lock.jsonto ensure compatibility or address issues with newer versions. [1] [2]