RAR5 (and maybe others) async methods are different by adamhathcock · Pull Request #1203 · adamhathcock/sharpcompress

adamhathcock · 2026-02-11T16:15:15Z

Yet the tests still pass. Probably need better tests

adamhathcock · 2026-02-11T16:55:05Z

@copilot see if you have enough to make RAR tests that exercise fully the Unpack5Async code path

Copilot · 2026-02-11T16:55:16Z

@adamhathcock I've opened a new pull request, #1205, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

This PR refactors RAR unpacking to better align async and sync implementations (motivated by #1201), primarily by moving large async implementations into dedicated *.Async.cs / *_async.cs partials and adjusting some unpack logic in the RAR5 path.

Changes:

Split multiple inline async unpack methods out of existing “cpp” translation units into new async partial-class files for both UnpackV2017 and UnpackV1.
Refactored some RAR5 (v2017) table/header reading helpers to use shared instance fields rather than passing ref state around.
Updated RAR5 unpack logic to use the 32-bit bitreader and correct copy behavior in fragmented window scenarios.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 39 comments.

Show a summary per file

File	Description
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack_cpp.cs	Minor async signature cleanup (Task/CancellationToken using directives).
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_cpp.cs	Removes embedded async methods and refactors block/table helpers to use instance fields.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs	Adds dedicated async RAR5 unpack implementation and async block/table readers.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_cpp.cs	Removes embedded async methods.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs	Adds dedicated async RAR2.x unpack implementation.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack15_cpp.cs	Removes embedded async methods.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack15_async.cs	Adds dedicated async RAR1.5 unpack implementation.
src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.cs	Normalizes async method signatures and CancellationToken usage.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack50.cs	Removes embedded async routines.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack50.Async.cs	Adds dedicated async routines for RAR5 unpack in v1 implementation.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack20.cs	Removes embedded async routines.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack20.Async.cs	Adds dedicated async routines for RAR2.x unpack in v1 implementation.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack15.cs	Removes embedded async routines.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack15.Async.cs	Adds dedicated async routines for RAR1.5 unpack in v1 implementation.
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.cs	Removes async entrypoints from the main file (moved to async partial).
src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.Async.cs	Adds async entrypoints and async implementations for v1 unpacking.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T17:01:50Z

src/SharpCompress/Compressors/Rar/UnpackV1/Unpack15.Async.cs

+            if (((wrPtr - unpPtr) & PackDef.MAXWINMASK) < 270 && wrPtr != unpPtr)
+            {
+                oldUnpWriteBuf();
+                if (suspended)
+                {
+                    return;
+                }
+            }


unpack15Async flushes using oldUnpWriteBuf() (sync Stream.Write) instead of the available async variant. This blocks the async extraction path and ignores the provided CancellationToken. Use await oldUnpWriteBufAsync(cancellationToken).ConfigureAwait(false) here.

Copilot · 2026-02-11T17:01:50Z

src/SharpCompress/Compressors/Rar/UnpackV1/Unpack.Async.cs

+            if (((wrPtr - unpPtr) & PackDef.MAXWINMASK) < 260 && wrPtr != unpPtr)
+            {
+                UnpWriteBuf();
+                if (destUnpSize < 0)
+                {
+                    return;
+                }
+                if (suspended)
+                {
+                    FileExtracted = false;
+                    return;
+                }
+            }


Inside Unpack29Async, the async code path flushes via UnpWriteBuf() (sync Stream.Write) even though UnpWriteBufAsync(...) is implemented just below and a CancellationToken is available. Consider switching this flush to await UnpWriteBufAsync(cancellationToken).ConfigureAwait(false) so async extraction doesn’t block and supports async-only write streams.

Copilot · 2026-02-11T17:01:51Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+            if (((WriteBorder - UnpPtr) & MaxWinMask) < MAX_LZ_MATCH + 3 && WriteBorder != UnpPtr)
+            {
+                await UnpWriteBufAsync(cancellationToken);
+                if (WrittenFileSize > DestUnpSize)
+                {
+                    return;
+                }


These awaits don’t use .ConfigureAwait(false) (await UnpWriteBufAsync(...)) while the rest of the async implementation consistently does. For library code this can cause unnecessary context capture. Consider using await UnpWriteBufAsync(cancellationToken).ConfigureAwait(false) here (and at the final flush).

Copilot · 2026-02-11T17:01:51Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+                continue;
+            }
+        }
+        await UnpWriteBufAsync(cancellationToken);


Final flush uses await UnpWriteBufAsync(cancellationToken); without .ConfigureAwait(false), which is inconsistent with the rest of the async code here. Consider adding .ConfigureAwait(false) to avoid capturing a synchronization context in library code.

Suggested change

await UnpWriteBufAsync(cancellationToken);

await UnpWriteBufAsync(cancellationToken).ConfigureAwait(false);

Copilot · 2026-02-11T17:01:51Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack20_async.cs

+            if (((WrPtr - UnpPtr) & MaxWinMask) < 270 && WrPtr != UnpPtr)
+            {
+                UnpWriteBuf20();
+                if (Suspended)
+                {
+                    return;
+                }
+            }


Unpack20Async flushes output via UnpWriteBuf20() (sync Stream.Write) even though this is the async code path and a CancellationToken is available. This defeats async I/O and can break with streams that only support async writes. Use await UnpWriteBuf20Async(cancellationToken).ConfigureAwait(false) here so writes are truly async and cancellable.

Copilot · 2026-02-11T17:02:00Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+        if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 16)
+        {
+            if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))
+            {
+                return false;
+            }


These 'if' statements can be combined.

Suggested change

if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 16)

{

if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

}

if (

!Inp.ExternalBuffer

&& Inp.InAddr > ReadTop - 16

&& !await UnpReadBufAsync(cancellationToken).ConfigureAwait(false)

)

{

return false;

Copilot · 2026-02-11T17:02:00Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+        if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 7)
+        {
+            if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))
+            {
+                return false;
+            }


These 'if' statements can be combined.

Suggested change

if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 7)

{

if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

}

if (!Inp.ExternalBuffer

&& Inp.InAddr > ReadTop - 7

&& !await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

Copilot · 2026-02-11T17:02:01Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+        if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 25)
+        {
+            if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))
+            {
+                return false;
+            }


These 'if' statements can be combined.

Suggested change

if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 25)

{

if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

}

if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 25 &&

!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

Copilot · 2026-02-11T17:02:01Z

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

+            if (!Inp.ExternalBuffer && Inp.InAddr > ReadTop - 5)
+            {
+                if (!await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))
+                {
+                    return false;
+                }
+            }


These 'if' statements can be combined.

src/SharpCompress/Compressors/Rar/UnpackV2017/Unpack.unpack50_async.cs

adamhathcock added 4 commits February 11, 2026 16:10

codex found problems

103ae60

divide async and sync logic

e786c00

validate and make sure rar5 methods are the same

218af5a

moved and validated more async code

cd5da3d

adamhathcock mentioned this pull request Feb 11, 2026

Async rar is very broken #1201

Closed

adamhathcock added 2 commits February 11, 2026 16:47

more moved and validated

ab1dd45

more cleaning

6f50545

adamhathcock marked this pull request as ready for review February 11, 2026 16:52

Copilot AI review requested due to automatic review settings February 11, 2026 16:52

Copilot started reviewing on behalf of adamhathcock February 11, 2026 16:53 View session

Copilot AI mentioned this pull request Feb 11, 2026

Add comprehensive RAR5 async extraction tests #1205

Closed

Copilot AI reviewed Feb 11, 2026

View reviewed changes

adamhathcock merged commit 33e9c78 into master Feb 11, 2026
12 checks passed

adamhathcock deleted the adam/issue-1201 branch February 11, 2026 17:41

	await UnpWriteBufAsync(cancellationToken);
	await UnpWriteBufAsync(cancellationToken).ConfigureAwait(false);

Conversation

adamhathcock commented Feb 11, 2026

Uh oh!

adamhathcock commented Feb 11, 2026

Uh oh!

Copilot AI commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments