[Microsoft.Android.Run] Await app shutdown on Ctrl+C - #12318
[Microsoft.Android.Run] Await app shutdown on Ctrl+C#12318jonathanpeppers wants to merge 3 commits into
Conversation
Ensure Ctrl+C waits for adb force-stop before the run tool exits, while preserving the standard cancellation exit code. Strengthen the device test to require shutdown completion without polling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace duplicated magic values with a documented constant. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a race where Microsoft.Android.Run could exit on Ctrl+C before adb shell am force-stop completed, leaving the app running on the device. It moves shutdown into the awaited run lifecycle and tightens the device integration test to require the app to be stopped when dotnet run exits.
Changes:
- Await
StopAppAsync()during the run lifecycle shutdown instead of fire-and-forget from the Ctrl+C handler. - Standardize SIGINT exit code handling via a
CtrlCExitCodeconstant and improveStopAppAsync()error reporting. - Strengthen
DotNetRunCtrlCdevice integration test to fail if cancellation is reported as an error and to require immediate app shutdown.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Microsoft.Android.Run/Program.cs |
Moves Ctrl+C shutdown work into the awaited lifecycle and adjusts stop-app behavior/exit code handling. |
tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs |
Updates Ctrl+C test expectations to validate no cancellation error and immediate device app termination. |
Track Ctrl+C independently from the cancellation token source because instrumentation also cancels that source during normal cleanup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 80fed9de-25fd-4f10-9c83-145a24798a3c
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The cleanup is now correctly awaited before exit, and the immediate device-state assertion is stronger. The awaited adb force-stop still needs a bounded shutdown path so an unresponsive adb cannot make Ctrl+C hang indefinitely; the regression test should also lock in exit code 130.
CI is still in progress; all completed checks currently pass.
Generated by Android PR Reviewer for #12318 · gpt56 · 160.9 AIC · ⌖ 10.1 AIC · ⊞ 25.3K
Comment /review to run again
| var userArg = string.IsNullOrEmpty (deviceUserId) ? "" : $" --user {deviceUserId}"; | ||
| await AdbHelper.RunAsync (adbPath, adbTarget, $"shell am force-stop{userArg} {package}", CancellationToken.None, verbose); | ||
| try { | ||
| var (exitCode, _, error) = await AdbHelper.RunAsync (adbPath, adbTarget, $"shell am force-stop{userArg} {package}", CancellationToken.None, verbose); |
There was a problem hiding this comment.
🤖 ProcessUtils.StartProcess waits for the process-exit event indefinitely, and OnCancelKeyPress keeps setting e.Cancel = true; if adb shell am force-stop hangs, dotnet run can no longer be stopped with Ctrl+C (including a second press). Please bound this cleanup with a timeout, or make a subsequent Ctrl+C terminate/cancel the in-flight cleanup.
Rule: Cancellation and timeout propagation
| string outputText = output.ToString (); | ||
| Assert.IsTrue (outputText.Contains ("Stopping application..."), | ||
| $"Output should contain 'Stopping application...' from Microsoft.Android.Run's Ctrl+C handler"); | ||
| Assert.IsFalse (outputText.Contains ("Error: The operation was canceled."), |
There was a problem hiding this comment.
🤖 💡 Testing — The production change promises exit code 130, but this test only checks output and process termination; it would pass if cancellation returned 0 or 1. Please assert process.ExitCode == 130 after WaitForExit so the behavior introduced with CtrlCExitCode is covered.
Rule: Verify observable regression behavior
Pressing Ctrl+C during
dotnet runcould letMicrosoft.Android.Runexit before its fire-and-forgetadb shell am force-stopcompleted, leaving the application running on the device.Move shutdown into the awaited run lifecycle. The Ctrl+C handler now only signals cancellation, while final cleanup waits for
force-stopbefore returning the standard SIGINT exit code. The device integration test now requires the app to be stopped immediately whendotnet runexits instead of polling around the race.Fixes #11264
DotNetRunCtrlC;Microsoft.Android.Runbuilds with 0 warnings and errors.