Fix emulator start false-positive when launcher forks and exits

[rmarinho]

Fixes the E2106 crash reported on maui android emulator start when the emulator actually boots successfully.

Repro

PS> dotnet run --project src\Cli\Microsoft.Maui.Cli --framework net10.0 -- \
      maui android emulator start MAUI_Emulator_API_36.1
Starting MAUI_Emulator_API_36.1... (3.1s)
Error [E2106]: Failed to start AVD 'MAUI_Emulator_API_36.1':
No process is associated with this object.

At this point adb devices shows nothing yet, but the emulator.exe and qemu-system-x86_64.exe processes ARE running — the emulator boots fine; the CLI just misreports the outcome.

Root cause

On Windows, emulator.exe is a launcher that forks qemu-system-*.exe and exits itself (typically with code 0) while the VM keeps running. With RedirectStandardOutput=true plus BeginOutputReadLine/BeginErrorReadLine active, the .NET runtime drops the managed Process handle shortly after the launcher exits. The next access to process.HasExited / process.ExitCode then throws InvalidOperationException: No process is associated with this object.

In AvdManager.StartAvdAsync, the 3-second post-launch health check did exactly that unguarded access, and the generic catch below wrapped it into the E2106 message.

The upstream EmulatorRunner.BootEmulatorAsync already handles this case:

Emulator launcher process exited with code 0 (likely forked). Continuing to poll adb devices.

Our fire-and-forget start path did not.

Fix

Wrap the HasExited / ExitCode check in a try/catch for InvalidOperationException. If the handle has been reaped we assume the launcher forked cleanly (matching upstream behavior). Only a confirmed non-zero exit code now surfaces as an error.

Validation

• Before the fix: Error [E2106]: ... No process is associated with this object every time, even though the emulator was up.
• After the fix: ✓ Started AVD: MAUI_Emulator_API_36.1 in 3.2s, adb devices shows emulator-5554 device.
• EmulatorStartTimingTests (2 tests) pass on Windows in ~8s.

Also includes the EmulatorStartTimingTests guardrail from the #122 investigation (opt-in via MAUI_TEST_AVD_NAME) — separately measures the adb says booted → start command returns gap so any future regression shows up as a test diff.

Related

• Investigation issue for the separate (pre-existing) 60s delay when the AVD was already booted: maui android emulator start: ~60s delay when AVD already booted (stale emulator console) #122

[Copilot]

Pull request overview

Fixes a Windows-specific false-positive failure in maui android emulator start where the emulator launcher (emulator.exe) forks and exits, causing .NET Process handle queries (HasExited/ExitCode) to throw and get misreported as an E2106 start failure.

Changes:

• Hardened the post-launch “immediate crash” check in AvdManager.StartAvdAsync to tolerate InvalidOperationException when the launcher process handle is no longer queryable, while still failing on confirmed non-zero exit codes.
• Added opt-in, live timing guardrail tests (MAUI_TEST_AVD_NAME) to measure the gap between adb-confirmed boot and StartAvdAsync completion for both already-booted and cold-start scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Cli/Microsoft.Maui.Cli/Providers/Android/AvdManager.cs	Avoids misreporting successful Windows fork/exit behavior as an emulator start failure by guarding Process exit checks.
src/Cli/Microsoft.Maui.Cli.UnitTests/EmulatorStartTimingTests.cs	Adds opt-in live tests to detect regressions in emulator-start return timing relative to adb boot confirmation.

[Copilot]

The XML doc says this test records the wall-clock time when adb first confirms boot (T_adbBooted) and then computes T_toolReturned - T_adbBooted, but the implementation currently just measures how long IsBootCompletedAsync takes (adbConfirmAtMs) and then measures StartAvdAsync duration with a separate stopwatch. Either update the methodology comment to match what’s actually being measured, or switch to a single shared stopwatch (or timestamp) so the reported “gap” is truly time-from-adb-confirmation to tool return.

[github-actions]

🔍 Expert Code Review — PR #131

Methodology: 3 independent reviewers with adversarial consensus. Findings included only when ≥2/3 reviewers agree (disputed findings were re-evaluated by the non-flagging reviewers).

---

🟡 MODERATE — catch (InvalidOperationException) is too broad — may mask real startup failures

File: src/Cli/Microsoft.Maui.Cli/Providers/Android/AvdManager.cs, lines 258–262 (in diff)
Consensus: 2/3 reviewers

Process.HasExited can throw InvalidOperationException for reasons other than a reaped handle — e.g., "No process Id has been set" when the process object was never successfully started. In that case launcherExited = true and launcherExitCode = null, so GetValueOrDefault() returns 0 — the error branch is skipped and StartAvdAsync returns as if the launch succeeded, even though no emulator is running.

Suggestion: Narrow the catch with an exception filter:

catch (InvalidOperationException ex)
    when (ex.Message.Contains("No process is associated", StringComparison.Ordinal))
{
    launcherExited = true;
}

---

🟡 MODERATE — RunAdbAsync deadlock pattern (read streams after WaitForExit)

File: src/Cli/Microsoft.Maui.Cli.UnitTests/EmulatorStartTimingTests.cs, lines 267–276 (in diff)
Consensus: 2/3 reviewers

RunAdbAsync calls await p.WaitForExitAsync(cts.Token) before reading StandardOutput/StandardError. If a future adb command produces output exceeding the OS pipe buffer (4–64 KB), the child blocks on write while WaitForExitAsync blocks on exit — classic deadlock. The timeout is the only escape, causing spurious (-1, "", "timeout") returns and silent test skips.

Suggestion: Read streams concurrently:

var stdoutTask = p.StandardOutput.ReadToEndAsync();
var stderrTask = p.StandardError.ReadToEndAsync();
await p.WaitForExitAsync(cts.Token);
var so = await stdoutTask;
var se = await stderrTask;

---

🟡 MODERATE — Race condition on adbBootedAtMs (non-atomic Nullable<long>)

File: src/Cli/Microsoft.Maui.Cli.UnitTests/EmulatorStartTimingTests.cs, line 131 (in diff)
Consensus: 3/3 reviewers (1 initially flagged, 2 confirmed on follow-up)

long? adbBootedAtMs is written by the poller task and read by the main thread with no synchronization. Nullable<long> is a two-field struct; writes are not atomic. If Task.Delay(1000) wins the WhenAny race before the poller completes, subsequent reads have no happens-before guarantee — a torn read is theoretically possible.

Suggestion: Use Interlocked.Exchange/Interlocked.Read on a plain long with a -1 sentinel, or await pollerTask after cts.Cancel() to establish ordering.

---

🟢 MINOR — Negative gap can't detect premature-return regression

File: src/Cli/Microsoft.Maui.Cli.UnitTests/EmulatorStartTimingTests.cs, lines 174–178 (in diff)
Consensus: 3/3 reviewers (1 initially flagged, 2 confirmed on follow-up)

In the cold-start test, gap = toolReturnedAtMs - adbBootedAtMs.Value. If StartAvdAsync(wait:true) returns before the poller observes boot, gap is negative and negative < 5000 trivially passes — the test silently misses a premature-return regression.

Suggestion: Assert both bounds:

Assert.True(gap >= -2000,
    $"StartAvdAsync returned {-gap} ms BEFORE adb confirmed boot.");
Assert.True(gap < AcceptableGap.TotalMilliseconds, ...);

---

🟢 MINOR — Opt-in tests report as "passed" instead of "skipped"

File: src/Cli/Microsoft.Maui.Cli.UnitTests/EmulatorStartTimingTests.cs, lines 52/57/69/113/116 (in diff)
Consensus: 3/3 reviewers (1 initially flagged, 2 confirmed on follow-up)

When MAUI_TEST_AVD_NAME is unset, both tests return early with no assertion. xUnit v2 reports these as passed, not skipped. CI dashboards show green tests that exercised nothing, hiding that the guardrail never actually ran.

Suggestion: Use Skip.If() or throw SkipException so test runners accurately report these as skipped.

---

📋 Summary

#	Severity	File	Consensus	Category
1	🟡 MODERATE	AvdManager.cs:258	2/3	Bug — broad exception catch masks failures
2	🟡 MODERATE	EmulatorStartTimingTests.cs:267	2/3	Bug — deadlock pattern in test helper
3	🟡 MODERATE	EmulatorStartTimingTests.cs:131	3/3	Race condition — unsynchronized shared state
4	🟢 MINOR	EmulatorStartTimingTests.cs:174	3/3	Test gap — one-sided assertion
5	🟢 MINOR	EmulatorStartTimingTests.cs:52	3/3	Test gap — silent pass vs skip

Discarded (single reviewer only): error message null literal, pre-existing process disposal, Process.Start null-forgiving operator, cold-start test teardown — each flagged by only 1/3 and not confirmed by the other two.

🏗️ CI Status

• ✅ CLA: passed
• 🔄 Build (macOS): in progress
• 🔄 Build (Windows): in progress

Overall Assessment

The core production fix in AvdManager.cs is correct and well-targeted — it properly handles the Windows emulator launcher fork/exit pattern that caused false E2106 errors. Finding #1 (broad catch) is the only production-code concern and is low risk in practice (the LaunchEmulator code path makes it unlikely HasExited would throw for other reasons), but narrowing the filter is a quick defensive improvement.

Findings #2–5 are all in the new test file. The tests are well-designed opt-in integration guardrails, but have minor robustness issues that could cause silent misreports in edge cases.

Generated by Expert Code Review (auto) for issue #131 · ● 7.3M · ◷