2024-10-06

F# で Cmdlet を書いてる pt.50

krymtkts/pocof 開発をした。 v0.16.0 と、そのバグ修正版の v0.16.1 をリリースしたのでその話を書く。

コードクォートでクエリの述語を実装してみた版を v0.16.0 でリリースした。 pocof は .NET Standard 2.0 を対象にしてるので PowerShell と Windows PowerShell の両方で動くのだけど、この版に含まれる変更に起因して Windows PowerShell でだけエラーになるバグ #235 が発生した。

> '' | pocof

# pocof : The startIndex argument must be greater than or equal to zero.
# Parameter name: startIndex
# At line:1 char:6
# + '' | pocof
# + ~~~~~
# + CategoryInfo : NotSpecified: (:) [Select-Pocof], ArgumentOutOfRangeException
# + FullyQualifiedErrorId : System.ArgumentOutOfRangeException,Pocof.SelectPocofCommand

Windows/Linux/MacOS の PowerShell ではエラーにならない。率直に Windows PowerShell になにか問題あるよなと考えた。

バグが入り込んだコミット krymtkts/pocof@2554092 は人力二分探索で特定した。 該当のコミットでの変更差分は以下の通り。

         member __.Receive() =
- match renderStack.Count with
- | 0 -> RenderMessage.None
- | c ->
- let items = Array.zeroCreate<RenderEvent> c
- renderStack.TryPopRange(items) |> ignore
- items |> Array.toList |> getLatestEvent
+ let items = Array.zeroCreate<RenderEvent> renderStack.Count
+ renderStack.TryPopRange(items) |> ignore
+ items |> Array.toList |> getLatestEvent

コレ見て分かる通り、 0 のケースを ConcurrentStack<T>.TryPopRange に寄せたのが良くなかったみたい。 PowerShell 7.4 と Windows PowerShell 5.1 はそれぞれ .NET 8 と .NET Framework 4.5 に依存しててたまに挙動の違いがあるからこういうこともあるよなという感じ。 今回はそこのテストが不十分だった。

このエラーは Windows PowerShell でも確認できる。依存する .NET/.NET Framework で動くからな。 (Windows PowerShell のほうが CLRVersion 4.0. ~ なのがよくわからんかったが CLRVersion は major version で一緒ということを知った)

# PowerShell の場合
> $PSVersionTable; $s = [System.Collections.Concurrent.ConcurrentStack[System.String]]::new(); $s.Count; $s.TryPopRange(@())

# Name Value
# ---- -----
# PSVersion 7.4.5
# PSEdition Core
# GitCommitId 7.4.5
# OS Microsoft Windows 10.0.22631
# Platform Win32NT
# PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
# PSRemotingProtocolVersion 2.3
# SerializationVersion 1.1.0.1
# WSManStackVersion 3.0
# 0
# 0
# Windows PowerShell の場合
> $PSVersionTable; $s = [System.Collections.Concurrent.ConcurrentStack[System.String]]::new(); $s.Count; $s.TryPopRange(@())

# Name Value
# ---- -----
# PSVersion 5.1.22621.4249
# PSEdition Desktop
# PSCompatibleVersions {1.0, 2.0, 3.0, 4.0...}
# BuildVersion 10.0.22621.4249
# CLRVersion 4.0.30319.42000
# WSManStackVersion 3.0
# PSRemotingProtocolVersion 2.3
# SerializationVersion 1.1.0.1
# 0
# Exception calling "TryPopRange" with "1" argument(s): "The startIndex argument must be greater than or equal to zero.
# Parameter name: startIndex"
# At line:1 char:104
# + ... ConcurrentStack[System.String]]::new(); $s.Count; $s.TryPopRange(@())
# + ~~~~~~~~~~~~~~~~~~~
# + CategoryInfo : NotSpecified: (:) [], MethodInvocationException
# + FullyQualifiedErrorId : ArgumentOutOfRangeException

とここまで追い込んだら修正するだけだったので、 #237 で修正して v0.16.1 を出した。


ここからはタダの brain dump だ。

直し方がわかって修正したらあとはこの動作の差異が何なのかというのが気になる。コードを見に行った。 microsoft/referencesource dotnet/runtime/ どちらもライセンスが MIT なので snippet を貼る。 以下は ConcurrentStack<T>.TryPopRange の引数の validation をしているメソッドだ。

.NET の方は runtime/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs から runtime/src/libraries/System.Collections/src/System/Collections/ThrowHelper.cs を呼ぶ。

        private static void ValidatePushPopRangeInput(T[] items, int startIndex, int count)
{
ArgumentNullException.ThrowIfNull(items);

ArgumentOutOfRangeException.ThrowIfNegative(count);

int length = items.Length;
ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex, length);
ArgumentOutOfRangeException.ThrowIfNegative(startIndex);

if (length - count < startIndex) //instead of (startIndex + count > items.Length) to prevent overflow
{
throw new ArgumentException(SR.ConcurrentStack_PushPopRange_InvalidCount);
}
}
        public static void ThrowIfGreaterThan<T>(T value, T other, [CallerArgumentExpression(nameof(value))] string? paramName = null)
where T : IComparable<T>
{
if (value.CompareTo(other) > 0)
{
ThrowGreater(value, other, paramName);
}
}

.NET Framework の方は referencesource/mscorlib/system/collections/Concurrent/ConcurrentStack.cs

        private void ValidatePushPopRangeInput(T[] items, int startIndex, int count)
{
if (items == null)
{
throw new ArgumentNullException("items");
}
if (count < 0)
{
throw new ArgumentOutOfRangeException("count", Environment.GetResourceString("ConcurrentStack_PushPopRange_CountOutOfRange"));
}
int length = items.Length;
if (startIndex >= length || startIndex < 0)
{
throw new ArgumentOutOfRangeException("startIndex", Environment.GetResourceString("ConcurrentStack_PushPopRange_StartOutOfRange"));
}
if (length - count < startIndex) //instead of (startIndex + count > items.Length) to prevent overflow
{
throw new ArgumentException(Environment.GetResourceString("ConcurrentStack_PushPopRange_InvalidCount"));
}
}

.NET の方で ArgumentOutOfRangeException.ThrowIfGreaterThan(startIndex, length) になったとき startIndex = length のケースがエラーの対象に含まれなくなってんのよね。

ドキュメントも見てきた。 .NET 8 が ConcurrentStack.TryPopRange Method (System.Collections.Concurrent) | Microsoft Learn 。 .NET Framework 4.5.2 が ConcurrentStack.TryPopRange Method (System.Collections.Concurrent) | Microsoft Learn

どっちも ArgumentOutOfRangeException にはこう書いてる。

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

これって実装がドキュメントの挙動と違うねんけど、どっちが間違ってんの?となって理解進んでないのが現状。いやさっさと聞きに行ったら良いねんけど、どっちが妥当なんか自分の中で腑に落ちてなくて立ち止まってるところ。 空の配列渡したら何の操作もなく戻る現状の挙動の方が使い良くて好きやけど、ちゃんとした裏付けしたいのでもうちょっと調べる。

例えばやで、仮にこう ↓ するとエラーにならんのよね。これは item.Length より大きい index を startIndex が指してるからやっぱおかしい気もするわ。 count = 0 の場合を特別扱いにするとかしないと辻褄あった API 仕様にならんのちゃうかなあ。

$s = [System.Collections.Concurrent.ConcurrentStack[System.String]]::new(); $s.Push("a"); $s.Count; $a = @($null); $s.TryPopRange(@('a'), 1, 0)
# 1
# 0

続く。