Launch emulator in signal-isolated process on Unix to survive Ctrl+C#361
Launch emulator in signal-isolated process on Unix to survive Ctrl+C#361
Conversation
…trl+C On Unix, wrap the emulator launch through /bin/sh with 'trap "" INT; exec ...' which sets SIGINT to SIG_IGN before exec'ing the emulator. POSIX guarantees SIG_IGN is preserved across exec, so the emulator process ignores SIGINT (Ctrl+C) from the parent terminal. Also fix pre-existing bug where emulatorProcess.ExitCode was accessed after Dispose() in BootEmulatorAsync. Agent-Logs-Url: https://github.com/dotnet/android-tools/sessions/9eb96c77-ebf5-45a1-8e00-d54a177d6ef3 Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
|
I investigated P/Invoke alternatives and none work cleanly:
The core issue is that .NET's The
See also: POSIX |
|
/review |
|
✅ Android Tools PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
This PR updates Xamarin.Android.Tools.AndroidSdk’s emulator launching behavior on Unix so the Android emulator won’t be terminated by Ctrl+C sent to the parent terminal’s process group, while keeping Windows behavior unchanged. It also tightens emulator boot failure reporting and adds targeted Unix-only tests to validate the SIGINT-survival and shell-quoting behavior.
Changes:
- On Unix, launch the emulator via
/bin/sh -c "trap '' INT; exec ..."and add POSIX-safe single-quote escaping viaShellQuote(). - Fix
BootEmulatorAsyncto avoid readingProcess.ExitCodeafter disposing the process handle. - Update/extend tests to validate SIGINT survival on Unix and correct handling of emulator paths containing single quotes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Xamarin.Android.Tools.AndroidSdk/Runners/EmulatorRunner.cs |
Wrap Unix emulator launch in a SIGINT-ignoring shell+exec command; fix exit-code capture before dispose; add POSIX shell quoting helper. |
tests/Xamarin.Android.Tools.AndroidSdk-Tests/EmulatorRunnerTests.cs |
Adjust launch-failure assertion to check ErrorKind; add Unix-only tests for SIGINT survival and quoting of paths containing '. |
There was a problem hiding this comment.
🤖 ✅ LGTM — Clean, well-reasoned change.
Summary: The SIGINT isolation via trap '' INT; exec is the correct POSIX approach, and the ShellQuote implementation uses the canonical single-quote escaping pattern. The ExitCode-after-Dispose() bug fix is a genuine catch. The test for SIGINT survival is well-designed — it verifies the core behavior by sending a real signal and checking the process remains alive.
Suggestions (non-blocking):
- 💡 Move the two new tests above the
// --- Helpers ---separator to keep test/helper sections distinct ⚠️ Add process kill logic to thefinallyblocks in both new tests to match the cleanup pattern used by existing tests and avoid potential process leaks in CI
| Severity | Count |
|---|---|
| ❌ Error | 0 |
| 1 (test cleanup) | |
| 💡 Suggestion | 2 |
Positive callouts:
- The
exectrick preserves the Process PID tracking while achieving signal isolation — avoids the complexity of P/Invoke or process group management - Saving
ExitCodebeforeDispose()prevents a realObjectDisposedException - Good test coverage: both the SIGINT survival and the single-quote-in-path edge case
Generated by Android Tools PR Reviewer for issue #361 · ● 2.3M
- Add POSIX spec link to the trap/exec code comment - Remove section separator comments (// --- Helpers --- etc.) - Assert kill exit code in SurvivesSigint test to prevent false positives - Add timeout to chmod.WaitForExit() to prevent CI hangs - Move process cleanup to finally blocks to avoid leaking processes in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Took origin/main InvalidOperationException guard around HasExited and the new InvalidEmulatorBinary_ReturnsLaunchFailed test. Kept this branch's SIGINT-survival and ShellQuote tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| [Test] | ||
| [Platform ("Linux,MacOsX")] | ||
| public void LaunchEmulator_SurvivesSigint () |
jonathanpeppers
left a comment
There was a problem hiding this comment.
I think this will fix this problem, but I'm open to other solutions than this trap command.
I couldn't get copilot to find an alternative:

Ctrl+C of
dotnet runkills the Android emulator because the terminal sends SIGINT to the entire process group, including the child emulator process.Changes
Signal isolation on Unix: Wrap emulator launch through
/bin/sh -c 'trap "" INT; exec ...'.trap '' INTsets SIGINT to SIG_IGN, which POSIX guarantees is preserved acrossexec. The emulator process ignores SIGINT from the parent terminal while remaining killable viaadb emu killor window close.execreplaces the shell so theProcessobject tracks the correct PID.Windows unchanged: Ctrl+C propagation works differently on Windows; no wrapper needed.
Bug fix:
BootEmulatorAsyncaccessedemulatorProcess.ExitCodeafterDispose()— save exit code before disposing.Shell quoting: Added
ShellQuote()for proper POSIX single-quote escaping of emulator path and arguments.Tests
LaunchEmulator_SurvivesSigint— sends SIGINT to the launched process, asserts it remains aliveShellQuote_EscapesSingleQuotes— verifies paths containing'work correctly