From e2dc985a182b7989e560df483a10f908f10fe119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Fri, 17 Jul 2026 22:39:41 +0200 Subject: [PATCH 1/2] Fix concurrent-import crash in Run.Parallel (thread-unsafe verb patch) VerbsPatcher.AllowShouldVerb reaches into PowerShell's process-global System.Management.Automation.Verbs verb dictionary and mutates it. That Dictionary is not thread-safe, so parallel worker Import-Module calls in Run.Parallel could corrupt it and throw IndexOutOfRange in set_Item. Serialize both the insert and the delayed removal behind a process-global lock and guard the insert with a ContainsKey check so only the first import mutates the shared dictionary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/csharp/Pester/VerbsPatcher.cs | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/csharp/Pester/VerbsPatcher.cs b/src/csharp/Pester/VerbsPatcher.cs index b56473175..123dc519f 100644 --- a/src/csharp/Pester/VerbsPatcher.cs +++ b/src/csharp/Pester/VerbsPatcher.cs @@ -15,6 +15,14 @@ public static class VerbsPatcher // Concurrent bag in case we start this multiple times, and god forbid in parallel. private static ConcurrentBag s_tasks = new ConcurrentBag(); + // s_validVerbs is PowerShell's own process-global static Dictionary (one instance per + // process, shared by every runspace). Dictionary is not thread-safe, so when + // Run.Parallel imports Pester from several ForEach-Object -Parallel runspaces at once, the + // concurrent writes can tear the internal bucket array and throw IndexOutOfRangeException. + // VerbsPatcher lives in Pester.dll, which is loaded once per process, so this static lock + // object is shared across all runspaces and serializes every mutation of that dictionary. + private static readonly object s_lock = new object(); + public static void AllowShouldVerb(int powershellVersion) { var should = "Should"; @@ -25,17 +33,31 @@ public static void AllowShouldVerb(int powershellVersion) // private static readonly Dictionary s_validVerbs; Dictionary validVerbs = (Dictionary)verbsField.GetValue(null); - // Overwrite when we call this multiple times. - validVerbs[should] = true; // The bool does not matter. + // Only the first import structurally mutates the shared dictionary; later parallel + // workers see the key already present and become no-ops. The lock prevents concurrent + // writes from corrupting the dictionary. Do not use TryAdd, it is not available on the + // net462 target. + lock (s_lock) + { + if (!validVerbs.ContainsKey(should)) + { + validVerbs[should] = true; // The bool does not matter. + } + } s_tasks.Add(Task.Run(async () => { await Task.Delay(5_000); try { - if (validVerbs.ContainsKey(should)) + // Serialize the removal behind the same process-global lock so it cannot race + // with a concurrent insert from another runspace. No await inside the lock. + lock (s_lock) { - validVerbs.Remove(should); + if (validVerbs.ContainsKey(should)) + { + validVerbs.Remove(should); + } } } catch { } From d338c4c20a52ae34e2ee7c3c1d0c8250549b5701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Sat, 18 Jul 2026 09:30:23 +0200 Subject: [PATCH 2/2] Explain why AllowShouldVerb does not double-check outside the lock Reading the shared Dictionary with an unlocked ContainsKey before taking the lock would race with the guarded insert (a resize could tear the bucket array during a concurrent read), so document that the lock-only form is intentional and that a correct fast path would need a separate volatile flag. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/csharp/Pester/VerbsPatcher.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/csharp/Pester/VerbsPatcher.cs b/src/csharp/Pester/VerbsPatcher.cs index 123dc519f..91d04c11c 100644 --- a/src/csharp/Pester/VerbsPatcher.cs +++ b/src/csharp/Pester/VerbsPatcher.cs @@ -37,6 +37,16 @@ public static void AllowShouldVerb(int powershellVersion) // workers see the key already present and become no-ops. The lock prevents concurrent // writes from corrupting the dictionary. Do not use TryAdd, it is not available on the // net462 target. + // + // We deliberately do NOT double-check with an unlocked ContainsKey before taking the + // lock. That would be an optimization to skip the lock once patched, but reading a + // plain Dictionary outside the lock while another runspace is inserting inside the lock + // is itself a data race: the insert can trigger an internal bucket-array resize, and a + // concurrent unlocked read can observe the torn array and throw IndexOutOfRange, the + // exact failure this lock exists to prevent. AllowShouldVerb runs once per runspace at + // import time and the lock only holds a ContainsKey plus at most one insert, so there + // is no contention worth optimizing here. A correct fast path would need a separate + // volatile flag rather than an unlocked read of this non-thread-safe dictionary. lock (s_lock) { if (!validVerbs.ContainsKey(should))