dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

test-flake: ScriptActorTests parks process-wide ScriptExecutionScheduler workers; leaks one permanently and starves other tests #16

New Issue
Closed
opened 2026-07-17 01:03:58 -04:00 by dohertj2 · 0 comments
dohertj2 commented 2026-07-17 01:03:58 -04:00
Owner
Copy Link
Copy Source

Summary

ScriptActorTests.ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation fails intermittently — 3 of 6 full-suite runs on a clean main, and it takes other tests down with it. The test parks worker threads of the process-wide ScriptExecutionScheduler singleton on an unbounded semaphore wait, and in exactly the case where it fails it leaks a worker thread permanently for the rest of the test run. Every later test that executes a script or alarm then competes for a smaller pool; one run failed 22 tests.

This is a test-harness defect, not a product defect. Found while verifying an unrelated change (Gitea #14); confirmed pre-existing by stashing that work.

Symptom

Assert.Equal() Failure: Values differ
Expected: 2
Actual:   1
   at ScriptActorTests.<>c.<ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation>b__18_1()
      in tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs:373
   at Akka.TestKit.TestKitBase.AwaitAssert(...)

The second evaluation never starts inside the 10 s AwaitAssert window.

Collateral failures in the same runs — script executions that never complete:

DeploymentManagerActorTests.RouteInboundApiCall_WithParentExecutionId_RoutesToScriptSuccessfully
DeploymentManagerActorTests.RouteInboundApiCall_WithoutParentExecutionId_StillRoutes
  Failed: Timeout 00:00:10 while waiting for a message of type
          ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi.RouteToCallResponse

Repro and rate

Full assembly, clean main:

dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests
Mode Result
Full suite, default parallelism 3 of 6 runs failed (1, 1 and 3 failures)
Full suite, later sample 2 of 4 failed
Full suite, xUnit.ParallelizeTestCollections=false still fails — 1 of 3, and separately 1 of 2; one run failed 22 tests in 5 m
The test alone (--filter) 6 of 6 passed

The filtered run passing 6/6 is why this hides: running alone does not reproduce it. (Same trap as #15.)

Root cause

ScriptExecutionScheduler is a process-wide static singleton with a fixed pool of dedicated threads — Shared() at src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs:41-53, "first caller wins", ScriptExecutionThreadCount default 8 (SiteRuntimeOptions.cs:46). Every script and alarm execution in the assembly runs on it: ScriptExecutionActor.cs:115, AlarmExecutionActor.cs:92, AlarmActor.cs:557. Nothing ever disposes or resets the singleton, so its pool is shared by the entire test run.

The test's trigger expression calls EvalGate.Block() (ScriptActorTests.cs:542-548):

public static class EvalGate
{
    public static readonly SemaphoreSlim Gate = new(0);
    private static int _entries;
    public static int Entries => Volatile.Read(ref _entries);
    public static bool Block() { Interlocked.Increment(ref _entries); Gate.Wait(); return false; }   // UNBOUNDED wait
}

Gate.Wait() on a 0-permit semaphore blocks the dedicated scheduler worker thread running the evaluation — by design, to hold _evalInFlight open. Two problems follow:

1. The pool is shared, so this starves other tests. While a worker is parked, every other concurrently-running test class executing a script competes for the remaining workers. That is what the RouteInboundApiCall_* timeouts are: a routed inbound-API script that never gets a worker within 10 s.

2. The failure case leaks a worker permanently. Teardown is:

finally { EvalGate.Gate.Release(EvalGate.Entries); }   // :377

It releases exactly Entries permits. In the failing run Entries == 1, so it releases one permit and the test exits — but the coalesced second evaluation is still queued on the scheduler. When a worker eventually dequeues it, Block() increments _entries to 2 and calls Gate.Wait() with no permits left and nothing alive to release them. That worker is parked for the remainder of the process, permanently shrinking the pool from 8. Repeat across a run and the pool degrades until many script-executing tests time out — the 22-failure run.

_entries is also static and never reset, so the counter carries across whatever ran before.

This is the same structural family as #15: a process-wide shared resource plus xUnit cross-class parallelism, with nothing serializing access.

Why "just disable parallelization" is not the fix

xUnit.ParallelizeTestCollections=false still fails (1/3, and 1/2 in a second sample) — the leaked worker outlives the test that leaked it, so sequential ordering does not help, and runtime triples (1 m 50 s → 5 m). Do not take that route.

Fix options

  1. (recommended) Make the gate un-leakable. Bound the wait — Gate.Wait(TimeSpan.FromSeconds(30)) — so a stranded evaluation can never park a worker for the rest of the run, and release generously in teardown rather than Release(Entries) (which under-releases in exactly the failure case). Reset _entries at test start. This keeps the test's intent (hold an evaluation in flight) while making its worst case self-healing.
  2. Isolate the scheduler. Give this test its own ScriptExecutionScheduler instance rather than the process-wide singleton. Needs an injection seam — ScriptExecutionActor/AlarmActor currently call ScriptExecutionScheduler.Shared(options) directly. Bigger change, but it removes the shared-pool coupling for all tests, not just this one.
  3. Serialize script-executing test classes into one xUnit collection (as HostBootCollection does for Host.Tests). Reduces contention but does not fix the permanent leak — option 1 or 2 is still required.

Option 1 is the smallest fix that addresses the actual failure. Option 2 is the structural one.

Adjacent risk (not the cause here)

ScriptExecutionScheduler.Shared is IDisposable, cached in a static that is never cleared. Nothing currently disposes it, but if anything ever did, _shared would keep handing out a disposed instance for the rest of the process and every subsequent script execution would fail. Worth a guard if the singleton stays.

Notes

  • Confirmed pre-existing: reproduced with all unrelated working-tree changes stashed.
  • Rates above are from a local run; CI hardware will shift them.
## Summary `ScriptActorTests.ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation` fails intermittently — **3 of 6** full-suite runs on a clean `main`, and it takes other tests down with it. The test parks worker threads of the **process-wide** `ScriptExecutionScheduler` singleton on an unbounded semaphore wait, and in exactly the case where it fails it **leaks a worker thread permanently for the rest of the test run**. Every later test that executes a script or alarm then competes for a smaller pool; one run failed **22 tests**. This is a **test-harness** defect, not a product defect. Found while verifying an unrelated change (Gitea #14); confirmed pre-existing by stashing that work. ## Symptom ``` Assert.Equal() Failure: Values differ Expected: 2 Actual: 1 at ScriptActorTests.<>c.<ExpressionEvalTaskFault_ClearsInFlight_AndDrainsPendingEvaluation>b__18_1() in tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/ScriptActorTests.cs:373 at Akka.TestKit.TestKitBase.AwaitAssert(...) ``` The second evaluation never starts inside the 10 s `AwaitAssert` window. Collateral failures in the same runs — script executions that never complete: ``` DeploymentManagerActorTests.RouteInboundApiCall_WithParentExecutionId_RoutesToScriptSuccessfully DeploymentManagerActorTests.RouteInboundApiCall_WithoutParentExecutionId_StillRoutes Failed: Timeout 00:00:10 while waiting for a message of type ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi.RouteToCallResponse ``` ## Repro and rate Full assembly, clean `main`: ``` dotnet test tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests ``` | Mode | Result | |---|---| | Full suite, default parallelism | **3 of 6 runs failed** (1, 1 and 3 failures) | | Full suite, later sample | 2 of 4 failed | | Full suite, `xUnit.ParallelizeTestCollections=false` | **still fails** — 1 of 3, and separately 1 of 2; one run failed **22** tests in 5 m | | The test alone (`--filter`) | **6 of 6 passed** | The filtered run passing 6/6 is why this hides: running alone does not reproduce it. (Same trap as #15.) ## Root cause `ScriptExecutionScheduler` is a **process-wide static singleton** with a fixed pool of dedicated threads — `Shared()` at `src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Scripts/ScriptExecutionScheduler.cs:41-53`, "first caller wins", `ScriptExecutionThreadCount` default **8** (`SiteRuntimeOptions.cs:46`). Every script and alarm execution in the assembly runs on it: `ScriptExecutionActor.cs:115`, `AlarmExecutionActor.cs:92`, `AlarmActor.cs:557`. Nothing ever disposes or resets the singleton, so its pool is shared by the entire test run. The test's trigger expression calls `EvalGate.Block()` (`ScriptActorTests.cs:542-548`): ```csharp public static class EvalGate { public static readonly SemaphoreSlim Gate = new(0); private static int _entries; public static int Entries => Volatile.Read(ref _entries); public static bool Block() { Interlocked.Increment(ref _entries); Gate.Wait(); return false; } // UNBOUNDED wait } ``` `Gate.Wait()` on a 0-permit semaphore blocks the **dedicated scheduler worker thread** running the evaluation — by design, to hold `_evalInFlight` open. Two problems follow: **1. The pool is shared, so this starves other tests.** While a worker is parked, every other concurrently-running test class executing a script competes for the remaining workers. That is what the `RouteInboundApiCall_*` timeouts are: a routed inbound-API script that never gets a worker within 10 s. **2. The failure case leaks a worker permanently.** Teardown is: ```csharp finally { EvalGate.Gate.Release(EvalGate.Entries); } // :377 ``` It releases exactly `Entries` permits. In the failing run `Entries == 1`, so it releases **one** permit and the test exits — but the coalesced second evaluation is still queued on the scheduler. When a worker eventually dequeues it, `Block()` increments `_entries` to 2 and calls `Gate.Wait()` with no permits left and nothing alive to release them. **That worker is parked for the remainder of the process**, permanently shrinking the pool from 8. Repeat across a run and the pool degrades until many script-executing tests time out — the 22-failure run. `_entries` is also static and never reset, so the counter carries across whatever ran before. This is the same structural family as #15: a **process-wide shared resource** plus xUnit cross-class parallelism, with nothing serializing access. ## Why "just disable parallelization" is not the fix `xUnit.ParallelizeTestCollections=false` **still fails** (1/3, and 1/2 in a second sample) — the leaked worker outlives the test that leaked it, so sequential ordering does not help, and runtime triples (1 m 50 s → 5 m). Do not take that route. ## Fix options 1. **(recommended) Make the gate un-leakable.** Bound the wait — `Gate.Wait(TimeSpan.FromSeconds(30))` — so a stranded evaluation can never park a worker for the rest of the run, and release generously in teardown rather than `Release(Entries)` (which under-releases in exactly the failure case). Reset `_entries` at test start. This keeps the test's intent (hold an evaluation in flight) while making its worst case self-healing. 2. **Isolate the scheduler.** Give this test its own `ScriptExecutionScheduler` instance rather than the process-wide singleton. Needs an injection seam — `ScriptExecutionActor`/`AlarmActor` currently call `ScriptExecutionScheduler.Shared(options)` directly. Bigger change, but it removes the shared-pool coupling for all tests, not just this one. 3. **Serialize script-executing test classes** into one xUnit collection (as `HostBootCollection` does for Host.Tests). Reduces contention but does **not** fix the permanent leak — option 1 or 2 is still required. Option 1 is the smallest fix that addresses the actual failure. Option 2 is the structural one. ## Adjacent risk (not the cause here) `ScriptExecutionScheduler.Shared` is `IDisposable`, cached in a static that is never cleared. Nothing currently disposes it, but if anything ever did, `_shared` would keep handing out a disposed instance for the rest of the process and every subsequent script execution would fail. Worth a guard if the singleton stays. ## Notes - Confirmed pre-existing: reproduced with all unrelated working-tree changes stashed. - Rates above are from a local run; CI hardware will shift them.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/ScadaBridge#16
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?