Surface still-running extensions during MTP shutdown (refs #5345)#8582

Merged

Evangelink merged 4 commits into

main from

dev/amauryleve/mtp-shutdown-progress

Jun 2, 2026

Merged

Surface still-running extensions during MTP shutdown (refs #5345) #8582
Evangelink merged 4 commits into
main from
dev/amauryleve/mtp-shutdown-progress

Conversation

@Evangelink

@Evangelink Evangelink commented May 25, 2026

Copy link

Copy Markdown

Member

Closes part of #5345 (incremental improvement, complementary to the phased-shutdown RFC in #8580).

Problem

Today when a user hits Ctrl+C on a hanging MTP test run, the cancellation token gets signalled but the user has no visibility into which extension or consumer is blocking the shutdown. The process just appears stuck.

Approach

Add a small IShutdownProgressReporter service that:

Tracks units of in-flight shutdown work via using (reporter.Track(uid, displayName, phase)) { await ... }.
Once the test-application cancellation token fires, waits a short quiet window (3s) to avoid noise on clean shutdowns.
Then periodically (every 1s) emits a Still waiting for: <Name1> (<Phase1>, <Ns>); <Name2> ... warning through IOutputDevice.

The three known blocking await sites are now wrapped:

ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (non-consumer pass) — CommonTestHost
ITestSessionLifetimeHandler.OnTestSessionFinishingAsync (consumer pass) — CommonTestHost
IAsyncConsumerDataProcessor.DrainDataAsync per consumer — AsynchronousMessageBus

This is intentionally complementary to #8580 . The RFC there proposes a full phased-shutdown protocol with a graceful timer and forced abort. The same trackers introduced here will feed the eventual Force-killed because X didn't drain message in that bigger work — but in the meantime this PR ships value independently with zero behavior change on the happy path.

Limitations

Does not surface slow user test code (only platform extensions/consumers expose Uid/DisplayName).
Output uses WarningMessageOutputDeviceData (yellow text); coordination with the live progress line is best-effort.
Strings are localized via PlatformResources.resx / regenerated .xlf files.

Tests

7 new unit tests in ShutdownProgressReporterTests covering snapshot semantics, idempotent dispose, quiet-window behavior, post-cancellation emission, no-emit-if-all-disposed, and post-disposal Track. All pass on net9.0 and net462.

Out of scope

Server-mode / IPC progress propagation.
CLI options to tune the quiet window or poll interval (constants for now).
Acceptance-test coverage (would require a long-sleeping fake extension; deferring until the RFC lands).

Copilot AI review requested due to automatic review settings

May 25, 2026 21:25

Copilot started reviewing on behalf of Evangelink

May 25, 2026 21:25

View session

Copilot AI reviewed

May 25, 2026

View reviewed changes

Copilot AI 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

Adds a shutdown-progress tracking/reporting service to Microsoft.Testing.Platform (MTP) so that after Ctrl+C cancellation, the host can periodically warn which extensions/consumers are still blocking shutdown.

Changes:

Introduces IShutdownProgressReporter + default ShutdownProgressReporter watchdog that emits "Still waiting for ..." warnings after a quiet window once cancellation is requested.
Wraps key shutdown await sites with tracking scopes (session finishing handlers + async consumer drain).
Adds unit tests and localized resource strings for the warning prefix.

Show a summary per file

File	Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/ShutdownProgressReporterTests.cs	Adds unit tests covering tracking/snapshot and watchdog emission behavior.
src/Platform/Microsoft.Testing.Platform/Services/IShutdownProgressReporter.cs	Introduces internal tracking interface for shutdown in-flight work.
src/Platform/Microsoft.Testing.Platform/Services/ShutdownProgressReporter.cs	Implements watchdog that reports still-running shutdown work via output device + logging.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.CommonServices.cs	Registers `ShutdownProgressReporter` in the common service provider (after output device).
src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs	Tracks `OnTestSessionFinishingAsync` awaits (non-consumer + consumer passes).
src/Platform/Microsoft.Testing.Platform/Messages/AsynchronousMessageBus.cs	Tracks per-consumer `DrainDataAsync` awaits; adds ctor overload to accept reporter.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostBuilder.Framework.cs	Passes `IShutdownProgressReporter` into `AsynchronousMessageBus` construction.
src/Platform/Microsoft.Testing.Platform/Hosts/TestHostControllersTestHost.cs	Passes `IShutdownProgressReporter` into `AsynchronousMessageBus` construction.
src/Platform/Microsoft.Testing.Platform/Resources/PlatformResources.resx	Adds localized resource key for "Still waiting for: " prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.cs.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.de.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.es.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.fr.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.it.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ja.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ko.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pl.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.pt-BR.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.ru.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.tr.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hans.xlf	Adds new trans-unit for the shutdown progress prefix.
src/Platform/Microsoft.Testing.Platform/Resources/xlf/PlatformResources.zh-Hant.xlf	Adds new trans-unit for the shutdown progress prefix.

Copilot's findings

Files reviewed: 22/22 changed files
Comments generated: 0

github-code-quality[bot]

github-code-quality Bot found potential problems

May 25, 2026

View reviewed changes

src/Platform/Microsoft.Testing.Platform/Services/ShutdownProgressReporter.cs Fixed

src/Platform/Microsoft.Testing.Platform/Services/ShutdownProgressReporter.cs Dismissed

@Evangelink

Evangelink commented Jun 1, 2026

Copy link

ばつ 2, drain ×ばつ 1). Doesn't cover slow user test code — fine as a starting point. - Constants only, no CLI knobs for the quiet window / poll interval — fine for v1. - Could be partly subsumed by #8580, but RFCs take time and this delivers value now. ### CodeQL bot comments - Three `catch` → `catch (Exception)` tightenings: applied in 5cf2741 and replied inline. - `catch (ObjectDisposedException)` in `Dispose()`: declined, replied inline — that pattern is idiomatic for a disposal race and the bot was suggesting we log noise without diagnostic value. ### Verification - `Microsoft.Testing.Platform.csproj` builds clean on net8.0, net9.0, netstandard2.0. - `Microsoft.Testing.Platform.UnitTests` (including the 7 new `ShutdownProgressReporterTests`) all pass on net8.0, net9.0, and net462." data-view-component="true"> Copy Markdown

Member Author

Self-review summary

I went through the PR end-to-end to weigh the added complexity against the value. My take: it's worth landing as an incremental improvement, with the minor catch-clause tightening I just pushed in 5cf2741.

Value vs. complexity

Solves a documented real-user pain point (CTRL+C does not complete the test run when there is long running test #5345 — opaque hangs on Ctrl+C).
Implementation is self-contained (~250 LOC + ~200 LOC tests), all-internal API (no public surface commitment).
Zero behavior change on the happy path: the watchdog only ticks once cancellation is requested and a 3s quiet window has elapsed.
Only 6 tiny touchpoints in existing code, via non-invasive using-block trackers — this is a sympathetic refactor, not an architectural shift.
The IShutdownProgressReporter trackers are explicitly reusable for Prototype: phased shutdown for MTP (#5345) #8580 's phased-shutdown work (per the PR body), so this is not throwaway scaffolding.

Caveats already disclosed in the PR body

Only 3 wrapped sites today (session-finishing ×ばつ 2, drain ×ばつ 1). Doesn't cover slow user test code — fine as a starting point.
Constants only, no CLI knobs for the quiet window / poll interval — fine for v1.
Could be partly subsumed by Prototype: phased shutdown for MTP (#5345) #8580 , but RFCs take time and this delivers value now.

CodeQL bot comments

Three catch → catch (Exception) tightenings: applied in 5cf2741 and replied inline.
catch (ObjectDisposedException) in Dispose(): declined, replied inline — that pattern is idiomatic for a disposal race and the bot was suggesting we log noise without diagnostic value.

Verification

Microsoft.Testing.Platform.csproj builds clean on net8.0, net9.0, netstandard2.0.
Microsoft.Testing.Platform.UnitTests (including the 7 new ShutdownProgressReporterTests) all pass on net8.0, net9.0, and net462.

@Evangelink Evangelink marked this pull request as ready for review

June 1, 2026 08:40

Copilot AI review requested due to automatic review settings

June 1, 2026 08:40

Copilot started reviewing on behalf of Evangelink

June 1, 2026 08:41

View session

Copilot AI reviewed

Jun 1, 2026

View reviewed changes

Copilot AI 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: 22/22 changed files
Comments generated: 1

src/Platform/Microsoft.Testing.Platform/Services/ShutdownProgressReporter.cs

Evangelink and others added 2 commits

June 1, 2026 10:46

@Evangelink


 Surface still-running extensions during MTP shutdown

5bbe074

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>

@Evangelink


 Make bare catch clauses explicit (catch (Exception))

0df243d

Addresses three CodeQL findings about generic catch clauses in the watchdog and ReportAsync of ShutdownProgressReporter. Behavior is intentionally preserved -- these are best-effort during shutdown -- so the only change is making the swallow explicit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@Evangelink Evangelink force-pushed the dev/amauryleve/mtp-shutdown-progress branch from 5cf2741 to 0df243d Compare

June 1, 2026 08:48

@Evangelink


 Avoid ObjectDisposedException race on _watchdogStopSource.Token

4f79dc1

Capture the watchdog stop token synchronously in OnCancellationRequested
and thread it through RunWatchdogAsync into ReportAsync, so a concurrent
Dispose() (which both cancels and disposes the CTS) cannot turn the
DisplayAsync call site into an ObjectDisposedException path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings

June 1, 2026 12:16

Copilot started reviewing on behalf of Evangelink

June 1, 2026 12:16

View session

Copilot AI reviewed

Jun 1, 2026

View reviewed changes

Copilot AI 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: 22/22 changed files
Comments generated: 2

src/Platform/Microsoft.Testing.Platform/Services/ShutdownProgressReporter.cs

test/UnitTests/Microsoft.Testing.Platform.UnitTests/Services/ShutdownProgressReporterTests.cs


 Keep watchdog alive when snapshot is empty after quiet window

5dbd63e

If cancellation happens before any shutdown work is tracked (e.g. extensions only start their shutdown handlers after the quiet window has elapsed), the watchdog must keep polling instead of exiting silently. Otherwise late tracking is never reported.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@Evangelink Evangelink enabled auto-merge (squash)

June 2, 2026 08:09

YuliiaKovalova

YuliiaKovalova approved these changes

Jun 2, 2026

View reviewed changes

@Evangelink Evangelink merged commit 8bcc2c5 into main

Jun 2, 2026

39 checks passed

@Evangelink Evangelink deleted the dev/amauryleve/mtp-shutdown-progress branch

June 2, 2026 08:32

Labels

None yet

4 participants

@Evangelink @YuliiaKovalova

Conversation

@Evangelink Evangelink commented May 25, 2026

Problem

Approach

Limitations

Tests

Out of scope

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Evangelink commented Jun 1, 2026

Self-review summary

Value vs. complexity

Caveats already disclosed in the PR body

CodeQL bot comments

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants