Skip to content

Parallel Pester 6 test random execution failure on TrisoftCmdletLogger static singleton #265

Description

@ddemeyer

Summary

When running Pester 6 with \Run.Parallel = \True, see #242, random test failures occur across different test files with errors such as:

  • NotImplementedException: WriteDebug
  • InvalidOperationException: Collection was modified after the enumerator was instantiated
  • InvalidOperationException: A 'break' or 'continue' statement with a label that does not match any enclosing loop escaped from your code

These failures are non-deterministic — different tests fail on different runs. They only appear in parallel mode, never in sequential mode.

Root Cause

Trisoft.ISHRemote.dll is a binary module loaded once per process and shared across all runspaces. In Pester 6 parallel mode, each test file runs in its own runspace via ForEach-Object -Parallel, but all runspaces share the same .NET AppDomain and therefore the same static fields.

TrisoftCmdletLogger is a process-wide singleton (TrisoftCmdletLogger.cs):

private static readonly TrisoftCmdletLogger _instance = new TrisoftCmdletLogger();
private static TrisoftCmdlet _cmdlet;  // ← shared across ALL runspaces

Every TrisoftCmdlet construction (i.e. every cmdlet invocation in any runspace) calls:

// TrisoftCmdlet.cs line 69
TrisoftCmdletLogger.Initialize(this);  // ← overwrites _cmdlet for the whole process

When Runspace A and Runspace B both invoke cmdlets concurrently, they race to overwrite _cmdlet. Runspace A's logger then calls _cmdlet.WriteDebug() but _cmdlet now points to Runspace B's cmdlet instance. Calling WriteDebug/WriteVerbose/WriteProgress on a PSCmdlet that belongs to a different runspace throws NotImplementedException or InvalidOperationException — which then surfaces as random test failures at unpredictable points in the call stack.

The picture:

Process (pwsh.exe) — one shared .NET heap
│
├── Trisoft.ISHRemote.dll  ← loaded ONCE, shared
│   └── TrisoftCmdletLogger._instance  (static singleton)
│       └── static TrisoftCmdlet _cmdlet  ← THE SHARED MUTABLE STATE
│
├── Runspace A  (Worker: AddIshDocumentObj.Tests.ps1)
│   └── new AddIshDocumentObj() cmdlet instantiated
│       → TrisoftCmdlet constructor:
│           TrisoftCmdletLogger.Initialize(this)  ← sets _cmdlet = cmdletA
│       → cmdletA calls IshTypeFieldSetup.ToIshMetadataFields(...)
│           → logger.WriteDebug(...)
│               → _cmdlet.WriteDebug(...)  ← _cmdlet is cmdletA ✓
│
├── Runspace B  (Worker: SetIshBaseline.Tests.ps1)  ← runs CONCURRENTLY
│   └── new SetIshBaseline() cmdlet instantiated
│       → TrisoftCmdlet constructor:
│           TrisoftCmdletLogger.Initialize(this)  ← sets _cmdlet = cmdletB
│                                                    OVERWRITES cmdletA ✗
│       → now Runspace A's logger.WriteDebug(...)
│           → _cmdlet.WriteDebug(...)
│               → cmdletB.WriteDebug(...)
│                   → NotImplementedException or InvalidOperationException
│                     because cmdletB belongs to a different runspace

Three Candidate Fixes

Option 1 — [ThreadStatic] on _cmdlet

Mark _cmdlet as [ThreadStatic] so each thread (each parallel runspace runs on its own thread) maintains its own pointer:

[ThreadStatic]
private static TrisoftCmdlet _cmdlet;

Pro: Minimal change — one attribute, no architectural impact, no ripple across callers.
Con: [ThreadStatic] fields are not initialized by the field initializer (only the first thread gets it); requires a null-check guard in every Write* method, which is already present. Also technically fragile if PowerShell ever reuses threads across runspaces (not currently the case for ForEach-Object -Parallel). Does not eliminate the singleton pattern — future maintainers may not realise it is thread-scoped.


Option 2 — Pass TrisoftCmdlet directly as ILogger through constructors, remove singleton [preferred]

Remove the singleton entirely. IshSession, IshTypeFieldSetup, and all helper classes that accept ILogger already do so through their constructors. The change is that the ILogger passed in is the TrisoftCmdlet instance itself (which already implements or wraps ILogger), rather than the process-global TrisoftCmdletLogger.Instance().

Pro: Architecturally correct — each cmdlet invocation carries its own logger scoped to its own runspace. No static mutable state. Parallel-safe by design, not by workaround. Works identically across net48, net6.0, and net10.0 because it relies on no threading primitives at all.
Con: Requires verifying every constructor call site that currently passes TrisoftCmdletLogger.Instance() and replacing it with this (the cmdlet) or a per-invocation logger wrapper. Medium-sized refactor across all cmdlet ProcessRecord/BeginProcessing methods.


Option 3 — [ThreadStatic] + null-safe no-op fallback (defensive hardening)

Same as Option 1 but explicitly replace the catch (PSInvalidOperationException) swallowing with a null guard before the call:

public void WriteDebug(string message)
{
    var cmdlet = _cmdlet;  // local copy avoids TOCTOU
    if (cmdlet == null) return;
    try { cmdlet.WriteDebug(message); }
    catch (PSInvalidOperationException) { }
}

Pro: Stops the cross-runspace PSCmdlet call entirely rather than letting it throw and catching. Slightly safer than bare [ThreadStatic] alone.
Con: Still a workaround around a fundamentally wrong design. Same long-term fragility concerns as Option 1.


Preferred Fix

Option 2 — remove the singleton, pass ILogger (the cmdlet instance) through constructors.

The singleton exists for historical convenience. That convenience is already gone — every IshTypeFieldSetup and IshSession constructor already accepts ILogger. The singleton only survives in TrisoftCmdlet.cs where Logger = TrisoftCmdletLogger.Instance() is assigned and \Initialize(this)\ is called immediately after. Replacing those two lines with a per-instance logger removes the static mutable state entirely and makes parallel safety a structural property rather than a threading annotation.

Options 1 / 3 are acceptable short-term mitigations if the refactor scope of Option 2 is too large for the current milestone.

Acceptance Criteria

  • All existing Pester tests pass sequentially (no regression)
  • Running Invoke-Pester with Run.Parallel = True and Run.ParallelThrottleLimit = 4 over Cmdlets/ produces zero random failures across 10 consecutive runs
  • No static TrisoftCmdlet _cmdlet field remains in the codebase

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions