You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2026/04/14 21:09:40 UTC
[PR] Fix Stream.ReadExactly CA2022 warning; optimize for stackalloc/arraypool [lucenenet]
paulirwin opened a new pull request, #1254:
URL: https://github.com/apache/lucenenet/pull/1254
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/master/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [X] You've included unit or integration tests for your change, where applicable.
- [X] You've included inline docs for your change, where applicable.
- [ ] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Fix the CA2022 code analysis warning, and add some related optimizations.
## Description
This fixes the CA2022 code analysis warning that started appearing in main Lucene.Net non-test code after the .NET 10 upgrade, saying that you should make sure to respect the return value of `Stream.Read` or you can use `Stream.ReadExactly`. Note that we started seeing this a while back when we added .NET 9 to the unit test projects (see #1052), but it did not flag this StreamExtensions code since the main library was still compiled with .NET 8 at the time.
For older platforms that don't support `Stream.ReadExactly`, this adds a polyfill, only for the `Span<T>` version, including unit tests. This is an improved version of the one that was originally proposed in (and then removed from) PR #1052, and simplified since it uses spans instead of arrays. While one valid approach could be to simply check the result of the Read call to address the warning, this at least lets us delegate to the BCL version for modern versions of .NET.
Also this corrects the feature flag for `Stream.ReadExactly` to correctly note that it was introduced in .NET 7, not .NET 9. The method is available for our .NET 8 target; it just wasn't a CA warning as of the .NET 8 SDK.
Finally, while I was in here I noticed several opportunities to reduce allocations. The small arrays of length 2-8 that were allocated on the heap previously are now stack-allocated for the synchronous methods. The asynchronous methods that weren't already using ArrayPool were updated to use that, since spans can't cross an await boundary. (A couple of the Write methods could be updated to use stack-allocated arrays instead of rented pool ones with C# 13, but that would require a further polyfill of WriteAsync, which could be done in a separate PR.) The synchronous methods are really only used by `Lucene.Net.Facet.Taxonomy.WriterCache.CharBlockArray`, and the asynchronous ones are used in `Lucene.Net.Replicator.SessionToken`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Stream.ReadExactly CA2022 warning; optimize for stackalloc/arraypool [lucenenet]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1254:
URL: https://github.com/apache/lucenenet/pull/1254#discussion_r3088922388
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -154,6 +157,76 @@ public static int Read(this Stream stream, Span<byte> buffer)
}
}
+#if !FEATURE_STREAM_READEXACTLY
+ /// <summary>
+ /// Reads bytes from the stream into <paramref name="buffer"/> until it is completely filled.
+ /// </summary>
+ /// <param name="stream">The stream to read from.</param>
+ /// <param name="buffer">The buffer to read into.</param>
+ /// <exception cref="ArgumentNullException"><paramref name="stream"/> is null.</exception>
+ /// <exception cref="EndOfStreamException">The end of the stream is reached before filling the buffer.</exception>
+ /// <remarks>
+ /// This method is a polyfill for platforms (prior to .NET 7) that do not have the ReadExactly method.
+ /// </remarks>
+ public static void ReadExactly(this Stream stream, Span<byte> buffer)
+ {
+ if (stream is null)
+ throw new ArgumentNullException(nameof(stream));
+
+ byte[] sharedBuffer = ArrayPool<byte>.Shared.Rent(buffer.Length);
+ try
+ {
+ while (buffer.Length > 0)
+ {
+ int numRead = stream.Read(sharedBuffer, 0, buffer.Length);
+
+ if (numRead == 0)
+ {
+ throw new EndOfStreamException(SR.IO_EOF_ReadBeyondEOF);
+ }
+
+ new ReadOnlySpan<byte>(sharedBuffer, 0, numRead).CopyTo(buffer);
+ buffer = buffer.Slice(numRead);
+ }
+ }
+ finally
+ {
+ ArrayPool<byte>.Shared.Return(sharedBuffer);
+ }
+ }
+
+ /// <summary>
+ /// Reads bytes asynchronously from the stream into <paramref name="buffer"/> until it is completely filled.
+ /// </summary>
+ /// <param name="stream">The stream to read from.</param>
+ /// <param name="buffer">The buffer to read into.</param>
+ /// <param name="offset">The byte offset in <paramref name="buffer"/> at which to begin writing data read from the stream.</param>
+ /// <param name="count">The count of bytes to read.</param>
+ /// <exception cref="ArgumentNullException"><paramref name="stream"/> is null.</exception>
+ /// <exception cref="EndOfStreamException">The end of the stream is reached before filling the buffer.</exception>
+ /// <remarks>
+ /// This method is a polyfill for platforms (prior to .NET 7) that do not have the ReadExactlyAsync method.
+ /// </remarks>
+ public static async Task ReadExactlyAsync(this Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken = default)
+ {
+ if (stream is null)
+ throw new ArgumentNullException(nameof(stream));
Review Comment:
The ReadExactlyAsync polyfill doesn't validate `buffer`, `offset`, or `count`. In particular, a negative `count` (or an `offset`/`count` range outside `buffer.Length`) will currently bypass the loop and silently succeed instead of throwing like the BCL ReadExactlyAsync overloads. Add argument validation to match Stream.ReadAsync/ReadExactlyAsync semantics (null/negative/range checks) before entering the loop.
```suggestion
throw new ArgumentNullException(nameof(stream));
if (buffer is null)
throw new ArgumentNullException(nameof(buffer));
if (offset < 0)
throw new ArgumentOutOfRangeException(nameof(offset));
if (count < 0)
throw new ArgumentOutOfRangeException(nameof(count));
if (buffer.Length - offset < count)
throw new ArgumentException("Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection.");
```
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -154,6 +157,76 @@ public static int Read(this Stream stream, Span<byte> buffer)
}
}
+#if !FEATURE_STREAM_READEXACTLY
+ /// <summary>
+ /// Reads bytes from the stream into <paramref name="buffer"/> until it is completely filled.
+ /// </summary>
+ /// <param name="stream">The stream to read from.</param>
+ /// <param name="buffer">The buffer to read into.</param>
+ /// <exception cref="ArgumentNullException"><paramref name="stream"/> is null.</exception>
+ /// <exception cref="EndOfStreamException">The end of the stream is reached before filling the buffer.</exception>
+ /// <remarks>
+ /// This method is a polyfill for platforms (prior to .NET 7) that do not have the ReadExactly method.
+ /// </remarks>
+ public static void ReadExactly(this Stream stream, Span<byte> buffer)
+ {
+ if (stream is null)
+ throw new ArgumentNullException(nameof(stream));
+
+ byte[] sharedBuffer = ArrayPool<byte>.Shared.Rent(buffer.Length);
+ try
+ {
+ while (buffer.Length > 0)
+ {
+ int numRead = stream.Read(sharedBuffer, 0, buffer.Length);
+
+ if (numRead == 0)
+ {
+ throw new EndOfStreamException(SR.IO_EOF_ReadBeyondEOF);
+ }
+
+ new ReadOnlySpan<byte>(sharedBuffer, 0, numRead).CopyTo(buffer);
+ buffer = buffer.Slice(numRead);
+ }
+ }
+ finally
+ {
+ ArrayPool<byte>.Shared.Return(sharedBuffer);
+ }
+ }
+
+ /// <summary>
+ /// Reads bytes asynchronously from the stream into <paramref name="buffer"/> until it is completely filled.
+ /// </summary>
+ /// <param name="stream">The stream to read from.</param>
+ /// <param name="buffer">The buffer to read into.</param>
+ /// <param name="offset">The byte offset in <paramref name="buffer"/> at which to begin writing data read from the stream.</param>
+ /// <param name="count">The count of bytes to read.</param>
+ /// <exception cref="ArgumentNullException"><paramref name="stream"/> is null.</exception>
+ /// <exception cref="EndOfStreamException">The end of the stream is reached before filling the buffer.</exception>
+ /// <remarks>
+ /// This method is a polyfill for platforms (prior to .NET 7) that do not have the ReadExactlyAsync method.
+ /// </remarks>
+ public static async Task ReadExactlyAsync(this Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken = default)
+ {
+ if (stream is null)
+ throw new ArgumentNullException(nameof(stream));
+
+ while (count > 0)
+ {
+ int numRead = await stream.ReadAsync(buffer, offset, count, cancellationToken);
Review Comment:
This await in the ReadExactlyAsync polyfill doesn't use ConfigureAwait(false), while the rest of this file's async methods consistently do. Add ConfigureAwait(false) here to avoid capturing a SynchronizationContext inside a library method.
```suggestion
int numRead = await stream.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Stream.ReadExactly CA2022 warning; optimize for stackalloc/arraypool [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1254:
URL: https://github.com/apache/lucenenet/pull/1254
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix Stream.ReadExactly CA2022 warning; optimize for stackalloc/arraypool [lucenenet]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1254:
URL: https://github.com/apache/lucenenet/pull/1254#discussion_r3082599490
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -366,17 +440,23 @@ public static async Task<string> ReadUTFAsync(this Stream input, CancellationTok
try
{
int readLen = await input.ReadAsync(lenBuff, 0, 2, cancellationToken).ConfigureAwait(false);
+
if (readLen < 2)
+ {
throw new EndOfStreamException("Unexpected end of stream while reading UTF length.");
+ }
Review Comment:
`ReadUTFAsync` reads the 2-byte length header with a single `ReadAsync(lenBuff, 0, 2, ...)`. Like other `ReadAsync` calls, this may return 1 byte without being EOF, which would currently throw even though more data may arrive. Consider using `ReadExactlyAsync` (or a small loop) to fill the 2-byte header reliably.
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -331,30 +383,52 @@ public static async Task<int> ReadInt32BigEndianAsync(this Stream input, Cancell
if (input is null)
throw new ArgumentNullException(nameof(input));
- byte[] buff = new byte[4];
- int read = await input.ReadAsync(buff, 0, buff.Length, cancellationToken).ConfigureAwait(false);
- if (read < buff.Length) throw new EndOfStreamException();
+ byte[] buff = ArrayPool<byte>.Shared.Rent(4);
+ try
+ {
+ int read = await input.ReadAsync(buff, 0, 4, cancellationToken).ConfigureAwait(false);
+
+ if (read < 4)
+ {
+ throw new EndOfStreamException();
+ }
Review Comment:
`ReadInt32BigEndianAsync` performs a single `ReadAsync` and throws if fewer than 4 bytes are returned. `Stream.ReadAsync` is allowed to return fewer bytes than requested even when not at EOF (e.g., network/crypto streams), so this can incorrectly throw. Please loop until 4 bytes are read or (on .NET 7+) delegate to `Stream.ReadExactlyAsync` (and consider a small polyfill for older TFMs).
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -331,30 +383,52 @@ public static async Task<int> ReadInt32BigEndianAsync(this Stream input, Cancell
if (input is null)
throw new ArgumentNullException(nameof(input));
- byte[] buff = new byte[4];
- int read = await input.ReadAsync(buff, 0, buff.Length, cancellationToken).ConfigureAwait(false);
- if (read < buff.Length) throw new EndOfStreamException();
+ byte[] buff = ArrayPool<byte>.Shared.Rent(4);
+ try
+ {
+ int read = await input.ReadAsync(buff, 0, 4, cancellationToken).ConfigureAwait(false);
+
+ if (read < 4)
+ {
+ throw new EndOfStreamException();
+ }
- return (buff[0] << 24) | (buff[1] << 16) | (buff[2] << 8) | buff[3];
+ return (buff[0] << 24) | (buff[1] << 16) | (buff[2] << 8) | buff[3];
+ }
+ finally
+ {
+ ArrayPool<byte>.Shared.Return(buff);
+ }
}
public static async Task<long> ReadInt64BigEndianAsync(this Stream input, CancellationToken cancellationToken = default)
{
if (input is null)
throw new ArgumentNullException(nameof(input));
- byte[] buff = new byte[8];
- int read = await input.ReadAsync(buff, 0, buff.Length, cancellationToken).ConfigureAwait(false);
- if (read < buff.Length) throw new EndOfStreamException();
-
- return ((long)buff[0] << 56) |
- ((long)buff[1] << 48) |
- ((long)buff[2] << 40) |
- ((long)buff[3] << 32) |
- ((long)buff[4] << 24) |
- ((long)buff[5] << 16) |
- ((long)buff[6] << 8) |
- buff[7];
+ byte[] buff = ArrayPool<byte>.Shared.Rent(8);
+ try
+ {
+ int read = await input.ReadAsync(buff, 0, 8, cancellationToken).ConfigureAwait(false);
+
+ if (read < 8)
+ {
+ throw new EndOfStreamException();
Review Comment:
`ReadInt64BigEndianAsync` assumes a single `ReadAsync` fills the requested 8 bytes and throws otherwise. Since `ReadAsync` may complete with a partial read without being EOF, this method can fail on valid streams. Please read in a loop until 8 bytes have been read (or use `ReadExactlyAsync` where available).
```suggestion
int offset = 0;
while (offset < 8)
{
int read = await input.ReadAsync(buff, offset, 8 - offset, cancellationToken).ConfigureAwait(false);
if (read == 0)
{
throw new EndOfStreamException();
}
offset += read;
```
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -366,17 +440,23 @@ public static async Task<string> ReadUTFAsync(this Stream input, CancellationTok
try
{
int readLen = await input.ReadAsync(lenBuff, 0, 2, cancellationToken).ConfigureAwait(false);
+
if (readLen < 2)
+ {
throw new EndOfStreamException("Unexpected end of stream while reading UTF length.");
+ }
int length = (lenBuff[0] << 8) | lenBuff[1];
byte[] buffer = ArrayPool<byte>.Shared.Rent(length);
try
{
int read = await input.ReadAsync(buffer, 0, length, cancellationToken).ConfigureAwait(false);
+
if (read < length)
+ {
throw new EndOfStreamException("Unexpected end of stream while reading UTF string.");
+ }
Review Comment:
`ReadUTFAsync` reads the UTF payload with a single `ReadAsync(buffer, 0, length, ...)` and throws if fewer than `length` bytes are returned. `ReadAsync` can legally return fewer bytes than requested without EOF, so this can fail on streams that chunk reads. Please loop until `length` bytes have been read (or use `ReadExactlyAsync` when available).
##########
src/Lucene.Net/Support/IO/StreamExtensions.cs:
##########
@@ -154,6 +157,32 @@ public static int Read(this Stream stream, Span<byte> buffer)
}
}
+#if !FEATURE_STREAM_READEXACTLY
+ /// <summary>
+ /// Reads bytes from the stream into <paramref name="buffer"/> until it is completely filled.
+ /// </summary>
+ /// <param name="stream">The stream to read from.</param>
+ /// <param name="buffer">The buffer to read into.</param>
+ /// <exception cref="ArgumentNullException"><paramref name="stream"/> is null.</exception>
+ /// <exception cref="EndOfStreamException">The end of the stream is reached before filling the buffer.</exception>
+ /// <remarks>
+ /// This method is a polyfill for platforms (prior to .NET 7) that do not have the ReadExactly method.
+ /// </remarks>
+ public static void ReadExactly(this Stream stream, Span<byte> buffer)
+ {
+ if (stream is null)
+ throw new ArgumentNullException(nameof(stream));
+
+ while (buffer.Length > 0)
+ {
+ int numRead = stream.Read(buffer);
+ if (numRead == 0)
+ throw new EndOfStreamException();
+ buffer = buffer.Slice(numRead);
+ }
Review Comment:
`ReadExactly(Span<byte>)` loops by repeatedly calling `stream.Read(buffer)`. On target frameworks where `Stream.Read(Span<byte>)` is *not* available, this binds to the `StreamExtensions.Read(Stream, Span<byte>)` polyfill which rents/returns an ArrayPool buffer per call. If the underlying stream returns partial reads, this can cause multiple ArrayPool rents in a tight loop. Consider renting a single temp buffer once (or using `Read(byte[], ...)` directly) and copying into the destination span until filled to avoid repeated pool traffic.
```suggestion
if (buffer.Length == 0)
return;
byte[] sharedBuffer = ArrayPool<byte>.Shared.Rent(buffer.Length);
try
{
int totalRead = 0;
while (totalRead < buffer.Length)
{
int remaining = buffer.Length - totalRead;
int numRead = stream.Read(sharedBuffer, 0, remaining);
if (numRead == 0)
throw new EndOfStreamException();
new ReadOnlySpan<byte>(sharedBuffer, 0, numRead).CopyTo(buffer.Slice(totalRead));
totalRead += numRead;
}
}
finally
{
ArrayPool<byte>.Shared.Return(sharedBuffer);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org