dotnet / dotnet-api-docs

.NET API reference documentation (.NET 5+, .NET Core, .NET Framework)
https://docs.microsoft.com/dotnet/api/
Other
725 stars 1.56k forks source link

`ConcurrentStack<T>.TryPopRange` does not throw `ArgumentOutOfRangeException` when `startIndex` equals the length of `items` #10508

Open krymtkts opened 3 weeks ago

krymtkts commented 3 weeks ago

Description

ConcurrentStack<T>.TryPopRange method does not throw an ArgumentOutOfRangeException when startIndex is equal to the length of items array. The documentation is as follows.

ConcurrentStack.TryPopRange Method (System.Collections.Concurrent) | Microsoft Learn>)

ArgumentOutOfRangeException startIndex or count is negative. Or startIndex is greater than or equal to the length of items.

This behavior is observed in .NET 8(PowerShell 7.4) but not in .NET Framework 4.5(Windows PowerShell 5.1), where the exception is correctly thrown.

.NET Framework checks whether the startIndex is equal to the length of items.

https://github.com/microsoft/referencesource/blob/51cf7850defa8a17d815b4700b67116e3fa283c2/mscorlib/system/collections/Concurrent/ConcurrentStack.cs#L473C1-L492

.NET doesn't.

https://github.com/dotnet/runtime/blob/48dbc4fc836da67cf1efb6b348499a918c4dea8e/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs#L390-L404

Reproduction Steps

Run the following code in PowerShell 7.4(.NET 8).

$s = [System.Collections.Concurrent.ConcurrentStack[System.String]]::new()
$a = [string[]]::new(0)
$s.TryPopRange($a) # TryPopRange(T[] items) calls TryPopRange(items, 0, items.Length)

$s.Push("a")
$b = [string[]]::new(1)
$s.TryPopRange($b, 1, 0)

Expected behavior

According to the documentation, ArgumentOutOfRangeException should be thrown when startIndex is equal to the length of items array.

Actual behavior

No exception is thrown.

Regression?

Yes, this behavior seems to be a regression from Use ArgumentOutOfRangeException.Throw helpers in more places (#79460) · dotnet/runtime@3689fbe.

Known Workarounds

Check startIndex and the length of items manually before calling TryPopRange.

Configuration

.NET 8.0.401

Other information

I noticed this issue when I passed an empty array to TryPopRange. Although it doesn't match the documentation, the current behavior of not throwing an error when passing an empty array is convenient. It might be better to implement an early return if the third argument count is 0 before ValidatePushPopRangeInput.

https://github.com/dotnet/runtime/blob/48dbc4fc836da67cf1efb6b348499a918c4dea8e/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs#L533-L548

dotnet-policy-service[bot] commented 3 weeks ago

Tagging subscribers to this area: @dotnet/area-system-collections See info in area-owners.md if you want to be subscribed.

skyoxZ commented 3 weeks ago

Seems intended: dotnet/runtime#62126

jeffhandley commented 3 weeks ago

Transferred this over to be an API docs issue to be fixed in xml/System.Collections.Concurrent/ConcurrentStack`1.xml. We should also update ConcurrentStack.cs (dotnet/runtime) to keep source and docs aligned. We just need appropriately clear wording to capture this nuance.

We should check PushRange's behavior to see if it also needs to be updated.

Marked as https://github.com/dotnet/dotnet-api-docs/labels/help%20wanted