Skip to content

Modernize Stream.ReadEx #1306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Carti-it wants to merge 3 commits into from
+1 −65
Closed

Modernize Stream.ReadEx #1306

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8df1d26
First attempt
Carti-it Jun 9, 2026
cedfb2f
Second attempt
Carti-it Jun 9, 2026
78ceee1
Remove preprocessor directive because Stream.ReadAsync seems to be su…
Carti-it Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 1 addition & 65 deletions NBitcoin/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ public static IEnumerable<List<T>> Partition<T>(this IEnumerable<T> source, Func
}
}

#if !NETSTANDARD1X

@Carti-it Carti-it Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is my understanding that .NET Standard support 1.x was removed in #1243 (commit 4780a7f).

public static int ReadEx(this Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellation = default(CancellationToken))
{
if (stream == null)
Expand All @@ -320,32 +319,7 @@ public static IEnumerable<List<T>> Partition<T>(this IEnumerable<T> source, Func

while (totalReadCount < count)
{
cancellation.ThrowIfCancellationRequested();

int currentReadCount;

//Big performance problem with BeginRead for other stream types than NetworkStream.
//Only take the slow path if cancellation is possible.
if (stream is NetworkStream && cancellation.CanBeCanceled)
{
var ar = stream.BeginRead(buffer, offset + totalReadCount, count - totalReadCount, null, null);
if (!ar.CompletedSynchronously)
{
WaitHandle.WaitAny(new WaitHandle[] { ar.AsyncWaitHandle, cancellation.WaitHandle }, -1);
}

//EndRead might block, so we need to test cancellation before calling it.
//This also is a bug because calling EndRead after BeginRead is contractually required.
//A potential fix is to use the ReadAsync API. Another fix is to register a callback with BeginRead that calls EndRead in all cases.
cancellation.ThrowIfCancellationRequested();

currentReadCount = stream.EndRead(ar);
}
else
{
//IO interruption not supported in this path.
currentReadCount = stream.Read(buffer, offset + totalReadCount, count - totalReadCount);
}
int currentReadCount = stream.ReadAsync(buffer, offset + totalReadCount, count - totalReadCount, cancellation).GetAwaiter().GetResult();

@Carti-it Carti-it Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://learn.microsoft.com/en-us/dotnet/api/system.io.stream.readasync?view=net-10.0 mentions that this overload is support on:

Technology Versions
.NET Core 1.0, 1.1, 2.0, 2.1, 2.2, 3.0, 3.1, 5, 6, 7, 8, 9, 10, 11
.NET Framework 4.5, 4.5.1, 4.5.2, 4.6, 4.6.1, 4.6.2, 4.7, 4.7.1, 4.7.2, 4.8, 4.8.1
.NET Standard 1.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 2.0, 2.1
UWP 10.0

My understanding is that those are all supported platforms for NBitcoin. To be sure, I checked https://www.nuget.org/packages/NBitcoin#supportedframeworks-body-tab as well. I don't see anything missing.

btw: I considered stream.Read() method but it does not support cancellation so I use stream.ReadAsync().


if (currentReadCount == 0)
return 0;
Expand All @@ -355,44 +329,6 @@ public static IEnumerable<List<T>> Partition<T>(this IEnumerable<T> source, Func

return totalReadCount;
}
#else

@Carti-it Carti-it Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that .NET Standard 1.x support was removed, the following code is dead code in my opinion.

@Carti-it Carti-it Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1307 attempts to remove the remaining .NET Standard 1.x code


public static int ReadEx(this Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellation = default(CancellationToken))
{
if(stream == null) throw new ArgumentNullException(nameof(stream));
if(buffer == null) throw new ArgumentNullException(nameof(buffer));
if(offset < 0 || offset > buffer.Length) throw new ArgumentOutOfRangeException("offset");
if(count <= 0 || count > buffer.Length) throw new ArgumentOutOfRangeException("count"); //Disallow 0 as a debugging aid.
if(offset > buffer.Length - count) throw new ArgumentOutOfRangeException("count");

//IO interruption not supported on these platforms.

int totalReadCount = 0;
#if !NOSOCKET
var interruptable = stream is NetworkStream && cancellation.CanBeCanceled;
#endif
while(totalReadCount < count)
{
cancellation.ThrowIfCancellationRequested();
int currentReadCount = 0;
#if !NOSOCKET
if(interruptable)
{
currentReadCount = stream.ReadAsync(buffer, offset + totalReadCount, count - totalReadCount, cancellation).GetAwaiter().GetResult();
}
else
#endif
{
currentReadCount = stream.Read(buffer, offset + totalReadCount, count - totalReadCount);
}
if(currentReadCount == 0)
return 0;
totalReadCount += currentReadCount;
}

return totalReadCount;
}
#endif

#if HAS_SPAN
public static int ReadEx(this Stream stream, Span<byte> buffer, CancellationToken cancellation = default(CancellationToken))
Expand Down
Loading