Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Prototype: phased shutdown for MTP (#5345) - #8580

Draft
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown
Draft

Prototype: phased shutdown for MTP (#5345)#8580
Amaury Levé (Evangelink) wants to merge 5 commits into
mainfrom
dev/amauryleve/mtp-phased-shutdown

Conversation

@Evangelink

Copy link
Copy Markdown
Member

Summary

Prototype of a two-phase graceful shutdown for Microsoft.Testing.Platform, exploring the design discussed in #5345 (see this comment).

Status: draft / RFC — opened for early design feedback. CLI options and most consumer migrations are intentionally deferred.

Motivation

Today MTP exposes a single ITestApplicationCancellationTokenSource.CancellationToken. On Ctrl+C, every consumer (tests, extensions, hosts) receives the same signal at the same time, and there is no contract distinguishing "please wind down gracefully" from "stop right now". This conflates the two shutdown modes that virtually every other host-style runtime treats as distinct phases (systemd EXTEND_TIMEOUT_USEC, macOS NSTerminateLater, AWS Lambda Extensions SHUTDOWN, Kubernetes preStop / terminationGracePeriodSeconds, docker compose 2-press SIGTERM→SIGKILL, Vitest teardownTimeout, …).

What's in this PR

A working prototype of Design A from the offline analysis:

ConceptImplementation
Two cancellation tokensDrainingToken (graceful) and AbortingToken (forceful) on the internal ITestApplicationCancellationTokenSource
Back-compatExisting CancellationToken is preserved as an alias of DrainingToken so the ~14 current consumers keep working unchanged
Abort() APIExplicit programmatic escalation entry point
State machineRunning → Draining → Aborting using Interlocked.CompareExchange for atomic phase transitions
Ctrl+C UX (matches docker compose / kubectl / npm)1st press → Draining (+ 30s grace timer); 2nd press → Aborting (+ 10s abort-timeout FailFast safety net); 3rd press → not intercepted, runtime kills the process
Auto-escalationGrace-period timer escalates Draining → Aborting if drain takes too long
Last-resortAbort-timeout uses IEnvironment.FailFast (banned Environment.FailFast replaced with the injectable wrapper)

Files

  • src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.cs — extended interface
  • src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs — full rewrite
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.cs — inline mock updated
  • test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs — 6 new unit tests

Verification

  • Builds clean on net8.0 / net9.0 / netstandard2.0
  • 6/6 new tests pass on net9.0
  • 6/6 new tests pass on net462 (confirms no DIM/runtime issues for the MSTest.TestAdapter consumer of the netstandard2.0 build)
  • 24/24 adjacent CommonHost / Cancellation / StopPolic* tests still pass — no regressions

Intentionally deferred (follow-ups)

To keep this PR reviewable, the following are explicitly NOT in scope and will land as separate PRs once the core direction is approved:

  • CLI options --shutdown-grace-period and --shutdown-abort-timeout via PlatformCommandLineProvider + HelpInfoTests acceptance assertions
  • Environment-variable propagation to TestHostOrchestratorHost / TestHostControllersTestHost controlled children
  • TerminalOutputDevice UX line: "Cancelling test session… (press Ctrl+C again to force quit)"
  • Migrating existing token consumers from the legacy alias onto explicit DrainingToken / AbortingToken usage
  • Coordinating with the HotReload extension's own CancelKeyPress handler
  • Design B (IShutdownParticipant ack/extend protocol) — separate RFC

Open design questions

  1. Should defaults be 30s/10s or come from HostOptions.ShutdownTimeout + a new AbortTimeout?
  2. Should the IEnvironment injection be threaded explicitly from TestHostBuilder.CommonServices now, or stay defaulted to SystemEnvironment?
  3. Are we happy with letting the 3rd Ctrl+C reach the runtime, or should we still Process.Kill ourselves?

Full RFC (motivation, prior-art table, rollout plan, alternatives) is available in my session notes and will be posted as a comment on #5345 once the prototype direction is acknowledged.

Refs #5345

Introduce a two-phase shutdown model for Microsoft.Testing.Platform so
test sessions get a deterministic drain window before being aborted:
- Extend internal ITestApplicationCancellationTokenSource with
DrainingToken (graceful cancel) and AbortingToken (forceful abort),
plus an Abort() entry point. The existing CancellationToken is kept
as a back-compat alias for DrainingToken so current consumers keep
observing graceful cancellation without changes.
- Rewrite CTRLPlusCCancellationTokenSource with a Running -> Draining
-> Aborting state machine:
* 1st Ctrl+C enters Draining, starts a 30s grace timer that
escalates to Aborting on elapse.
* 2nd Ctrl+C escalates to Aborting immediately, starts a 10s
abort-timeout safety net that FailFasts via IEnvironment.
* 3rd Ctrl+C is no longer intercepted - the runtime terminates
the process (matches docker compose / kubectl / npm UX).
- Add 6 unit tests covering initial state, single/idempotent cancel,
abort, grace-period escalation, and zero-grace escalation. Passes
on net9.0 and net462.
CLI options (--shutdown-grace-period, --shutdown-abort-timeout), env-
var propagation to controlled hosts, TerminalOutputDevice UX updates,
and migration of existing token consumers are intentionally deferred
to follow-up PRs and tracked in the RFC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prototype implementation of a two-phase shutdown model for Microsoft.Testing.Platform (MTP), introducing distinct “Draining” vs “Aborting” cancellation semantics to address Ctrl+C behavior discussed in #5345 while keeping back-compat for existing token consumers.

Changes:

  • Extend ITestApplicationCancellationTokenSource with DrainingToken, AbortingToken, and an Abort() escalation API (with CancellationToken preserved as a Draining alias).
  • Rewrite CTRLPlusCCancellationTokenSource to implement a Running → Draining → Aborting phase model with Ctrl+C escalation and grace/abort timers.
  • Add unit tests covering the new phase/token behavior.
Show a summary per file
FileDescription
src/Platform/Microsoft.Testing.Platform/Services/ITestApplicationCancellationTokenSource.csAdds two-phase shutdown tokens and Abort() API while preserving legacy token alias.
src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.csImplements the phased shutdown state machine, Ctrl+C escalation logic, and timers.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Hosts/CommonHostTests.csUpdates local test stub to satisfy the extended cancellation token source interface.
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.csAdds new unit tests validating draining/aborting semantics and grace escalation.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 5

Amaury Levé (Evangelink) added a commit that referenced this pull request Jun 1, 2026
When the test-application cancellation token is signalled and shutdown takes longer than expected, MTP now periodically prints a 'Still waiting for: ...' warning listing the extensions and consumers that have not yet returned. This makes a hanging Ctrl+C observable without users having to inspect the process state.
Adds a new internal IShutdownProgressReporter service plus a default ShutdownProgressReporter implementation. The reporter wraps the three known blocking await sites:
* ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (both non-consumer and consumer passes in CommonTestHost)
* IAsyncConsumerDataProcessor.DrainDataAsync per consumer in AsynchronousMessageBus
The watchdog only starts after the cancellation token fires, waits a quiet window (3s) to avoid noise on clean shutdowns, then polls every second. Output goes through IOutputDevice as a WarningMessageOutputDeviceData.
Refs #5345. Complementary to #8580 - the same trackers will feed the eventual `force-killed because X didn't drain` message once the phased-shutdown RFC lands.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Dispose() now detaches the Console.CancelKeyPress handler and disposes
any pending escalation CancellationTokenSources so callbacks don't fire
after the source is disposed and we don't keep the instance alive
through the static console event.
- OnConsoleCancelKeyPressed guards against post-Dispose invocations and
routes the 2nd Ctrl+C through Abort() rather than EnterAborting()
directly, keeping the Running -> Draining -> Aborting invariant.
- EnterAborting() now calls EnterDraining() first so the legacy
CancellationToken / DrainingToken are always signaled when AbortingToken
is canceled, and uses CompareExchange(Draining -> Aborting) instead of
an unconditional Exchange.
- ScheduleEscalation retains the timer CancellationTokenSource so it can
be disposed in Dispose().
- Test GracePeriodElapse_EscalatesToAborting is now event-driven via a
TaskCompletionSource registered on AbortingToken (no more Task.Delay
polling loop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink

Copy link
Copy Markdown
MemberAuthor

Copilot resolve the merge conflicts in this pull request

…ased-shutdown
# Conflicts:
#	src/Platform/Microsoft.Testing.Platform/Services/CTRLPlusCCancellationTokenSource.cs
#	test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/CTRLPlusCCancellationTokenSourceTests.cs
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 5, 2026 12:23
@Evangelink
Amaury Levé (Evangelink) removed the request for review from CopilotJune 5, 2026 12:23

CopilotAI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Done. Merged origin/main into this branch (commit 069d9ad). The two conflicts were in:

  • CTRLPlusCCancellationTokenSource.cs — resolved by keeping our two-phase implementation
  • CTRLPlusCCancellationTokenSourceTests.cs — resolved by keeping our phased-shutdown tests

Add coverage for Console.CancelKeyPress unsubscription and keep the phase field mutable while using the newer Lock type where available.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings June 7, 2026 08:00

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 4/4 changed files
  • Comments generated: 3

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink