Skip to content
Merged
Changes from all commits
Commits
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
40 changes: 36 additions & 4 deletions src/csharp/Pester/VerbsPatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Task> s_tasks = new ConcurrentBag<Task>();

// s_validVerbs is PowerShell's own process-global static Dictionary (one instance per
// process, shared by every runspace). Dictionary<TKey,TValue> 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";
Expand All @@ -25,17 +33,41 @@ public static void AllowShouldVerb(int powershellVersion)

// private static readonly Dictionary<string, bool> s_validVerbs;
Dictionary<string, bool> validVerbs = (Dictionary<string, bool>)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.
//
// 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))
{
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 { }
Expand Down
Loading