Add Async RAR and more by adamhathcock · Pull Request #996 · adamhathcock/sharpcompress

adamhathcock · 2025-10-29T09:03:42Z

Part of #992

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull Request Overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 58 comments.

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

Copilot · 2025-10-29T13:24:10Z

src/SharpCompress/Common/EntryStream.cs

-        {
-            return;
-        }
        _isDisposed = true;


Duplicate assignment of _isDisposed = true. The field is already set to true on line 103, making this second assignment redundant and potentially confusing.

Suggested change

_isDisposed = true;

Copilot · 2025-10-29T13:24:10Z

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

        }

-        var NewWindow = Fragmented ? null : new byte[WinSize];
+        var NewWindow = Fragmented ? null : ArrayPool<byte>.Shared.Rent((int)WinSize);


ArrayPool.Shared.Rent() may return an array larger than requested. The code should track the actual WinSize separately and ensure only the required portion is used, or clear the entire rented array to avoid using uninitialized memory from the pool.

Suggested change

var NewWindow = Fragmented ? null : ArrayPool<byte>.Shared.Rent((int)WinSize);

var NewWindow = Fragmented ? null : ArrayPool<byte>.Shared.Rent((int)WinSize);

if (NewWindow != null)

{

// Clear only the portion of the array that will be used.

Array.Clear(NewWindow, 0, (int)WinSize);

}

Copilot · 2025-10-29T13:24:11Z

src/SharpCompress/Compressors/Rar/RarCrcStream.cs

+        var array = System.Buffers.ArrayPool<byte>.Shared.Rent(buffer.Length);
+        try
+        {
+            var result = await base.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);
+            if (result != 0)
+            {
+                currentCrc = RarCRC.CheckCrc(currentCrc, buffer.Span, 0, result);
+            }
+            else if (
+                !disableCRC
+                && GetCrc() != BitConverter.ToUInt32(readStream.CurrentCrc, 0)
+                && buffer.Length != 0
+            )
+            {
+                // NOTE: we use the last FileHeader in a multipart volume to check CRC
+                throw new InvalidFormatException("file crc mismatch");
+            }
+
+            return result;
+        }
+        finally
+        {
+            System.Buffers.ArrayPool<byte>.Shared.Return(array);
+        }


The rented array is not used. The code rents an array but then calls base.ReadAsync(buffer, ...) with the original buffer, making the array allocation wasteful. Either use the rented array for the read operation or remove the unnecessary allocation.

Suggested change

var array = System.Buffers.ArrayPool<byte>.Shared.Rent(buffer.Length);

try

{

var result = await base.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);

if (result != 0)

{

currentCrc = RarCRC.CheckCrc(currentCrc, buffer.Span, 0, result);

}

else if (

!disableCRC

&& GetCrc() != BitConverter.ToUInt32(readStream.CurrentCrc, 0)

&& buffer.Length != 0

)

{

// NOTE: we use the last FileHeader in a multipart volume to check CRC

throw new InvalidFormatException("file crc mismatch");

}

return result;

}

finally

{

System.Buffers.ArrayPool<byte>.Shared.Return(array);

}

var result = await base.ReadAsync(buffer, cancellationToken).ConfigureAwait(false);

if (result != 0)

{

currentCrc = RarCRC.CheckCrc(currentCrc, buffer.Span, 0, result);

}

else if (

!disableCRC

&& GetCrc() != BitConverter.ToUInt32(readStream.CurrentCrc, 0)

&& buffer.Length != 0

)

{

// NOTE: we use the last FileHeader in a multipart volume to check CRC

throw new InvalidFormatException("file crc mismatch");

}

return result;

tests/SharpCompress.Test/Rar/RarArchiveAsyncTests.cs

Copilot · 2025-10-29T13:24:24Z

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

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


These 'if' statements can be combined.

Suggested change

if (Inp.InAddr > ReadTop - 25)

{

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

{

return false;

}

if (Inp.InAddr > ReadTop - 25 && !await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

Copilot · 2025-10-29T13:24:24Z

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

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


These 'if' statements can be combined.

Suggested change

if (Inp.InAddr > ReadTop - 5)

{

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

{

return false;

}

if (Inp.InAddr > ReadTop - 5 && !await UnpReadBufAsync(cancellationToken).ConfigureAwait(false))

{

return false;

Copilot · 2025-10-29T13:24:24Z

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

+                        var fltj = Filters[J];
+                        if (
+                            fltj.Type != FILTER_NONE
+                            && fltj.NextWindow == false


The expression 'A == false' can be simplified to '!A'.

Suggested change

&& fltj.NextWindow == false

&& !fltj.NextWindow

Copilot · 2025-10-29T13:24:25Z

tests/SharpCompress.Test/Rar/RarReaderAsyncTests.cs

+                if (!reader.Entry.IsDirectory)
+                {
+                    Assert.Equal(CompressionType.Rar, reader.Entry.CompressionType);
+                    var entryStream = await reader.OpenEntryStreamAsync();


This variable is manually disposed in a finally block - consider a C# using statement as a preferable resource management technique.

Copilot · 2025-10-29T13:24:25Z

src/SharpCompress/Archives/Rar/RarArchiveEntry.cs

+        RarStream stream;
+        if (IsRarV3)
+        {
+            stream = new RarStream(
+                archive.UnpackV1.Value,
+                FileHeader,
+                new MultiVolumeReadOnlyStream(Parts.Cast<RarFilePart>(), archive)
+            );
+        }
+        else
+        {
+            stream = new RarStream(
+                archive.UnpackV2017.Value,
+                FileHeader,
+                new MultiVolumeReadOnlyStream(Parts.Cast<RarFilePart>(), archive)
+            );
+        }


Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Suggested change

RarStream stream;

if (IsRarV3)

{

stream = new RarStream(

archive.UnpackV1.Value,

FileHeader,

new MultiVolumeReadOnlyStream(Parts.Cast<RarFilePart>(), archive)

);

}

else

{

stream = new RarStream(

archive.UnpackV2017.Value,

FileHeader,

new MultiVolumeReadOnlyStream(Parts.Cast<RarFilePart>(), archive)

);

}

RarStream stream = new RarStream(

IsRarV3 ? archive.UnpackV1.Value : archive.UnpackV2017.Value,

FileHeader,

new MultiVolumeReadOnlyStream(Parts.Cast<RarFilePart>(), archive)

);

adamhathcock · 2025-10-29T13:25:38Z

@copilot figure out why the windows tests fail

Copilot · 2025-10-29T13:25:51Z

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

adamhathcock added 19 commits October 28, 2025 15:00

basic async usage

3d29c18

first pass of async files

1ba529a

add async reads

b23f031

add async creations

8e7d959

add rarcrc tests

aca97c2

entrystream fixes and fmt

bb53d1e

fix logic mistake

b354f7a

fix RarStream

df2ed1e

add archive tests

a0c5b1c

convert unpack15 and unpack20

351e294

added more async methods

aa4cd37

async UnpReadBufAsync

16543bf

unpack50async

a09327b

async unpack1.5

1af51aa

async unpack2.0

58bab0d

fix async usage

b83e6ee

remove unused code

8324114

ran out of tokens, things work but need one more conversion

665d8cd

fmt

dc31e4c

Copilot AI review requested due to automatic review settings October 29, 2025 10:27

add arraypool usage in init

f238be6

Copilot AI reviewed Oct 29, 2025

View reviewed changes

add async for UnpWriteBufAsync and remove comments

65e6074

adamhathcock requested a review from Copilot October 29, 2025 13:12

add async tests for compress stream

75ada56

Copilot AI reviewed Oct 29, 2025

View reviewed changes

Initial plan

e786e95

Copilot AI mentioned this pull request Oct 29, 2025

Fix Windows test failures due to ArrayPool buffer sizing #1000

Merged

This was referenced Jan 12, 2026

Bump the main-dependencies group with 2 updates zencq/libNOM.io#121

Closed

update: Bump SharpCompress from 0.40.0 to 0.44.0 ethan-hann/CyberRadio-Assistant#116

Closed

This was referenced Jan 19, 2026

Bump SharpCompress from 0.39.0 to 0.44.1 AssetRipper/Tpk#119

Merged

update: Bump SharpCompress from 0.40.0 to 0.44.1 ethan-hann/CyberRadio-Assistant#117

Closed

Bump the main-dependencies group with 2 updates zencq/libNOM.io#122

Closed

This was referenced Jan 26, 2026

update: Bump SharpCompress from 0.40.0 to 0.44.4 ethan-hann/CyberRadio-Assistant#120

Closed

Bump the main-dependencies group with 2 updates zencq/libNOM.io#123

Open

This was referenced Feb 13, 2026

Bump SharpCompress from 0.39.0 to 0.46.0 tomtastisch/FileClassifier#36

Closed

update: Bump SharpCompress from 0.40.0 to 0.46.1 ethan-hann/CyberRadio-Assistant#122

Closed

This was referenced Feb 23, 2026

update: Bump SharpCompress from 0.40.0 to 0.46.2 ethan-hann/CyberRadio-Assistant#126

Closed

update: Bump SharpCompress from 0.40.0 to 0.46.4 ethan-hann/CyberRadio-Assistant#127

Closed

This was referenced Mar 4, 2026

Bump SharpCompress from 0.39.0 to 0.47.0 jas88/PACSify#121

Closed

update: Bump SharpCompress from 0.40.0 to 0.47.0 ethan-hann/CyberRadio-Assistant#128

Open

fix(deps): Bump SharpCompress from 0.39.0 to 0.47.0 nzbdav-dev/nzbdav#339

Open

-        var NewWindow = Fragmented ? null : ArrayPool<byte>.Shared.Rent((int)WinSize);
+        var NewWindow = Fragmented ? null : ArrayPool<byte>.Shared.Rent((int)WinSize);
+        if (NewWindow != null)
+        {
+            // Clear only the portion of the array that will be used.
+            Array.Clear(NewWindow, 0, (int)WinSize);
+        }

Conversation

adamhathcock commented Oct 29, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

adamhathcock commented Oct 29, 2025

Uh oh!

Copilot AI commented Oct 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants