Skip to content

Parallel Pester 6 test random execution failure on IshSession._ishTypeFieldSetup lazy init and setter are not thread-safe causing random Collection was modified failures #266

Description

@ddemeyer

Summary

When running Pester 6 with Run.Parallel = True, random test failures occur with:

InvalidOperationException: Collection was modified after the enumerator was instantiated.

The error surfaces at unpredictable test locations across different runs, for example:

[-] Parameter Left is TriDKXmlSetupFilePath and Right is IshSession using IncludeIdentical 3ms
    InvalidOperationException: Collection was modified after the enumerator was instantiated.
    at <ScriptBlock>, Cmdlets\Settings\CompareIshTypeFieldDefinition.Tests.ps1:108

This is a separate and independent defect from the TrisoftCmdletLogger singleton fixed in #265. That fix eliminated the cross-runspace PSCmdlet stream call race; this issue addresses the remaining data race on IshSession._ishTypeFieldSetup.

Root Cause

IshSession holds a lazily-initialised field:

private IshTypeFieldSetup _ishTypeFieldSetup;

Two code paths mutate this field without any synchronisation:

Path 1 — lazy init getter (IshSession.cs ~line 289):

internal IshTypeFieldSetup IshTypeFieldSetup
{
    get
    {
        if (_ishTypeFieldSetup == null)   // ← unsynchronised null-check
        {
            // ... construct new IshTypeFieldSetup, mutates _ishTypeFieldSetup
        }
        return _ishTypeFieldSetup;
    }
}

**Path 2 — IshTypeFieldDefinition\ setter** (IshSession.cs~line 478), called byGetIshTypeFieldDefinition.cs:113`:

internal set
{
    _ishTypeFieldSetup = new IshTypeFieldSetup(_logger, value);  // ← unsynchronised write
}

IshTypeFieldSetup._ishTypeFieldDefinitions is a SortedDictionary<string, IshTypeFieldDefinition>. .Values.ToList() — called from the getter IshTypeFieldDefinition => _ishTypeFieldDefinitions.Values.ToList() — opens an enumerator on that dictionary. If another parallel worker's cmdlet construction path triggers the IshTypeFieldSetup constructor (which calls _ishTypeFieldDefinitions.Add(...)) on the same dictionary instance concurrently, the enumerator throws Collection was modified.

In Pester 6 parallel mode, each worker runs in its own runspace but all runspaces share the same .NET AppDomain heap. While each worker creates its own IshSession via New-IshSession inside its BeforeAll, the Pester 6 parallel tape-replay mechanism passes live object references between the parent and worker runspaces without serialisation (as documented in the Pester source). This means a live IshSession reference created in one context can be read concurrently from another, making the unsynchronised field mutations unsafe.

The result is random — it depends on which test files land in the same worker batch and whether they happen to call Get-IshTypeFieldDefinition (which triggers the setter) while another caller is enumerating the dictionary.

Affected Code

File Location Issue
Objects/Public/IshSession.cs IshTypeFieldSetup getter (~line 289) Unsynchronised double-checked lazy init
Objects/Public/IshSession.cs IshTypeFieldDefinition setter (~line 478) Unsynchronised write to _ishTypeFieldSetup

Proposed Fix

Add a dedicated lock object to \IshSession\ and apply it to both the getter and the setter. This is a minimal, targeted change — 6 lines in IshSession.cs only, no API changes, no test changes, compatible with net48, net6.0, and net10.0.

// Add alongside the existing _ishTypeFieldSetup field declaration
private readonly object _ishTypeFieldSetupLock = new object();

Getter — wrap the existing null-check body:

internal IshTypeFieldSetup IshTypeFieldSetup
{
    get
    {
        if (_ishTypeFieldSetup == null)
        {
            lock (_ishTypeFieldSetupLock)
            {
                if (_ishTypeFieldSetup == null)  // double-checked lock
                {
                    // ... existing construction logic unchanged ...
                }
            }
        }
        return _ishTypeFieldSetup;
    }
}

Setter — protect the assignment:

internal set
{
    lock (_ishTypeFieldSetupLock)
    {
        _ishTypeFieldSetup = new IshTypeFieldSetup(_logger, value);
    }
}

Acceptance Criteria

  • Get-IshTypeFieldDefinition and Compare-IshTypeFieldDefinition tests pass consistently under Run.Parallel = True with Run.ParallelThrottleLimit = 4 across 10 consecutive runs
  • No Collection was modified errors in any parallel test run
  • Sequential test runs show no regression
  • No static or process-wide shared state introduced

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions