Skip to content

Hotfix: Program.cs ProcessExit-after-CTS-Dispose regression (every short-lived CLI exits 134)

Placeholder ppxd requested to merge fix/program-cts-disposal-regression into main

Summary

  • CRITICAL hotfix for a regression introduced in #274's Program.cs refactor
  • Every short-lived CLI invocation (version, show-thumbprint, show-config, etc.) returns exit code 134 instead of 0 due to ObjectDisposedException in the ProcessExit handler
  • Caught by the Linux smoke test Binary_Version_PrintsBuildVersionStamp running against PR #275/#276/#277/#278/#279/#280's CI

The bug

// PR #274's broken code:
using var consoleCts = new CancellationTokenSource();
Console.CancelKeyPress += (_, e) => { e.Cancel = true; consoleCts.Cancel(); };
AppDomain.CurrentDomain.ProcessExit += (_, _) => consoleCts.Cancel();

return await TentacleEntry.RunAsync(args, consoleCts.Token).ConfigureAwait(false);

What happens on squid-tentacle version:

  1. version command runs, prints 99.99.99
  2. try block returns → using var disposes consoleCts
  3. finally runs Log.CloseAndFlushAsync
  4. Process is terminating; ProcessExit handler fires
  5. Handler calls consoleCts.Cancel() on disposed CTS
  6. → ObjectDisposedException
  7. → unhandled exception
  8. → process aborts with exit 134 instead of 0

Operator impact

Every short-lived CLI invocation returns 134:

  • version (used by upgrade scripts to verify post-swap version)
  • show-thumbprint (used by operators / install-tentacle.sh)
  • show-config, list-instances, new-certificate
  • register's success path

Operator tooling that checks exit codes treats EVERY CLI call as failed.

CI evidence

PR #275, #276, #277, #278, #279, #280 ALL failed their first runner because they rebased on this broken main. The Linux smoke test caught it with the exact failure shape:

Got exit 134
Unhandled exception. System.ObjectDisposedException: The CancellationTokenSource has been disposed.
   at Program...b__2(Object _, EventArgs _) in Program.cs:line 59

Fix

// Post-fix:
var consoleCts = new CancellationTokenSource();   // NO `using` — match original (pre-#274)
Console.CancelKeyPress += (_, e) =>
{
    e.Cancel = true;
    try { consoleCts.Cancel(); }
    catch (ObjectDisposedException) { /* race vs shutdown */ }
};
AppDomain.CurrentDomain.ProcessExit += (_, _) =>
{
    try { consoleCts.Cancel(); }
    catch (ObjectDisposedException) { /* race vs shutdown */ }
};

Two-layer fix:

  1. Drop using var — match the original (pre-#274) Program.cs which used a non-using CTS that lives until process exit. Dispose-on-process-exit is automatic; explicit dispose was actively harmful.
  2. ObjectDisposedException catch in both handlers — defence-in-depth against any future code that disposes the CTS earlier.

Multi-line comment documents the rationale + historical context inline so future refactor attempts see the rule.

Why PR #274's CI didn't catch this

  • PR #274's tests focused on SCM-detection seam unit tests (16 cases) + the SCM-launched path (uses Host.RunAsync, NOT the consoleCts path)
  • The console-mode path was "unchanged" from the refactor's perspective; the using var was newly introduced as a "cleanup improvement" without realizing the static event handlers outlive the try block
  • Unit tests don't run the binary as a subprocess
  • The full E2E smoke test Binary_Version_PrintsBuildVersionStamp runs only on Tentacle Linux/Windows E2E workflows; PR #274 only triggered Windows E2E (its scope was Windows SCM), and the Windows version of the smoke test must have been running but somehow didn't catch — investigation will follow up

Going forward, every Tentacle production-binary change MUST trigger BOTH Linux + Windows E2E workflows. Will add to the contribution checklist.

Test plan

  • dotnet build — 0 errors
  • dotnet test VersionCommandTests — 2/2 passing
  • CI on ubuntu-latest Tentacle Linux E2E — Binary_Version_PrintsBuildVersionStamp passes (was the canary)
  • CI on windows-latest Tentacle Windows E2E — same smoke test on Windows side passes
  • All standard PR checks green (UnitTests, Calamari, Tentacle Tests, Integration)

After this lands

Re-trigger CI on the 6 P0/P1 PRs (#275, #276, #277, #278, #279, #280). They were ALL blocked on this regression; should green up cleanly post-merge.

🤖 Generated with Claude Code

Merge request reports

Loading