Skip to content

Commit 2bea0eb

Browse files
rmarinhormarinho
authored
Fix AdbRunner AVD detection: use getprop first, skip offline emulators (#312)
Two fixes for AdbRunner emulator detection reliability: 1. Swap AVD name detection order in GetEmulatorAvdNameAsync: use 'adb shell getprop ro.boot.qemu.avd_name' first (reliable on all modern emulators), fall back to 'adb emu avd name' for older versions. The 'emu avd name' command returns empty output on emulator 36.4.10+. 2. Skip AVD name queries for offline emulators in ListDevicesAsync: neither getprop nor 'emu avd name' works on offline devices, causing unnecessary delays during boot polling loops. Context: dotnet/android#10965
1 parent afb540a commit 2bea0eb

File tree

2 files changed

+170
-23
lines changed

2 files changed

+170
-23
lines changed

‎src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs‎

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public class AdbRunner
3636
/// </summary>
3737
/// <param name="adbPath">Full path to the adb executable (e.g., "/path/to/sdk/platform-tools/adb").</param>
3838
/// <param name="environmentVariables">Optional environment variables to pass to adb processes.</param>
39-
/// <param name="logger">Optional logger callback for diagnostic messages.</param>
39+
/// <param name="logger">Optional logger callback receiving a <see cref="TraceLevel"/> and message string.</param>
4040
public AdbRunner (string adbPath, IDictionary<string, string>? environmentVariables = null, Action<TraceLevel, string>? logger = null)
4141
{
4242
if (string.IsNullOrWhiteSpace (adbPath))
@@ -48,7 +48,8 @@ public AdbRunner (string adbPath, IDictionary<string, string>? environmentVariab
4848

4949
/// <summary>
5050
/// Lists connected devices using 'adb devices -l'.
51-
/// For emulators, queries the AVD name using 'adb -s &lt;serial&gt; emu avd name'.
51+
/// For online emulators, queries the AVD name via <c>getprop</c> / <c>emu avd name</c>.
52+
/// Offline emulators are included but without AVD names (querying them would fail).
5253
/// </summary>
5354
public virtual async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (CancellationToken cancellationToken = default)
5455
{
@@ -61,11 +62,15 @@ public virtual async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (Cancel
6162

6263
var devices = ParseAdbDevicesOutput (stdout.ToString ().Split ('\n'));
6364

64-
// For each emulator, try to get the AVD name
65+
// For each online emulator, try to get the AVD name.
66+
// Skip offline emulators — neither getprop nor 'emu avd name' work on them
67+
// and attempting these commands causes unnecessary delays during boot polling.
6568
foreach (var device in devices) {
66-
if (device.Type == AdbDeviceType.Emulator) {
69+
if (device.Type == AdbDeviceType.Emulator && device.Status == AdbDeviceStatus.Online) {
6770
device.AvdName = await GetEmulatorAvdNameAsync (device.Serial, cancellationToken).ConfigureAwait (false);
6871
device.Description = BuildDeviceDescription (device);
72+
} else if (device.Type == AdbDeviceType.Emulator) {
73+
logger.Invoke (TraceLevel.Verbose, $"Skipping AVD name query for {device.Status} emulator {device.Serial}");
6974
}
7075
}
7176

@@ -74,15 +79,26 @@ public virtual async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (Cancel
7479

7580
/// <summary>
7681
/// Queries the emulator for its AVD name.
77-
/// Tries <c>adb -s &lt;serial&gt; emu avd name</c> first (emulator console protocol),
78-
/// then falls back to <c>adb shell getprop ro.boot.qemu.avd_name</c> which reads the
79-
/// boot property set by the emulator kernel. The fallback is needed because
80-
/// <c>emu avd name</c> can return empty output on some adb/emulator version
81-
/// combinations (observed with adb v36).
82+
/// Tries <c>adb shell getprop ro.boot.qemu.avd_name</c> first (reliable on all modern
83+
/// emulators), then falls back to <c>adb -s &lt;serial&gt; emu avd name</c> (emulator
84+
/// console protocol) for older emulator versions. The <c>emu avd name</c> command returns
85+
/// empty output on emulator 36.4.10+ (observed with adb v36), so <c>getprop</c> is the
86+
/// preferred method.
8287
/// </summary>
8388
internal async Task<string?> GetEmulatorAvdNameAsync (string serial, CancellationToken cancellationToken = default)
8489
{
85-
// Try 1: Console command (fast, works on most emulator versions)
90+
// Try 1: Shell property (reliable on modern emulators, always set by the emulator kernel)
91+
try {
92+
var avdName = await GetShellPropertyAsync (serial, "ro.boot.qemu.avd_name", cancellationToken).ConfigureAwait (false);
93+
if (avdName is { Length: > 0 } name && !string.IsNullOrWhiteSpace (name))
94+
return name.Trim ();
95+
} catch (OperationCanceledException) {
96+
throw;
97+
} catch (Exception ex) {
98+
logger.Invoke (TraceLevel.Warning, $"GetEmulatorAvdNameAsync: getprop failed for {serial}: {ex.Message}");
99+
}
100+
101+
// Try 2: Console command (fallback for older emulators where getprop may not be available)
86102
try {
87103
using var stdout = new StringWriter ();
88104
var psi = ProcessUtils.CreateProcessStartInfo (adbPath, "-s", serial, "emu", "avd", "name");
@@ -97,19 +113,8 @@ public virtual async Task<IReadOnlyList<AdbDeviceInfo>> ListDevicesAsync (Cancel
97113
}
98114
} catch (OperationCanceledException) {
99115
throw;
100-
} catch {
101-
// Fall through to getprop fallback
102-
}
103-
104-
// Try 2: Shell property (fallback when 'adb emu avd name' returns empty on some adb/emulator versions)
105-
try {
106-
var avdName = await GetShellPropertyAsync (serial, "ro.boot.qemu.avd_name", cancellationToken).ConfigureAwait (false);
107-
if (avdName is { Length: > 0 } name && !string.IsNullOrWhiteSpace (name))
108-
return name.Trim ();
109-
} catch (OperationCanceledException) {
110-
throw;
111-
} catch {
112-
// Both methods failed
116+
} catch (Exception ex) {
117+
logger.Invoke (TraceLevel.Warning, $"GetEmulatorAvdNameAsync: both methods failed for {serial}: {ex.Message}");
113118
}
114119

115120
return null;

‎tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Linq;
8+
using System.Threading.Tasks;
89
using NUnit.Framework;
910

1011
namespace Xamarin.Android.Tools.Tests;
@@ -714,4 +715,145 @@ public void FirstNonEmptyLine_PmPathOutput ()
714715
var output = "package:/system/framework/framework-res.apk\n";
715716
Assert.AreEqual ("package:/system/framework/framework-res.apk", AdbRunner.FirstNonEmptyLine (output));
716717
}
718+
719+
// --- GetEmulatorAvdNameAsync + ListDevicesAsync tests ---
720+
// These tests use a fake 'adb' script to control process output,
721+
// verifying AVD detection order and offline emulator handling.
722+
723+
static string CreateFakeAdb (string scriptBody)
724+
{
725+
if (OS.IsWindows)
726+
Assert.Ignore ("Fake adb tests use bash scripts and are not supported on Windows.");
727+
728+
var dir = Path.Combine (Path.GetTempPath (), $"fake-adb-{Guid.NewGuid ():N}");
729+
Directory.CreateDirectory (dir);
730+
var path = Path.Combine (dir, "adb");
731+
File.WriteAllText (path, "#!/bin/bash\n" + scriptBody);
732+
FileUtil.Chmod (path, 0x1ED); // 0755
733+
734+
return path;
735+
}
736+
737+
static void CleanupFakeAdb (string adbPath)
738+
{
739+
var dir = Path.GetDirectoryName (adbPath);
740+
if (dir is { Length: > 0 }) {
741+
File.Delete (adbPath);
742+
Directory.Delete (dir);
743+
}
744+
}
745+
746+
[Test]
747+
public async Task GetEmulatorAvdNameAsync_PrefersGetprop ()
748+
{
749+
// getprop returns a value — should be used, emu avd name should NOT be needed
750+
var adbPath = CreateFakeAdb ("""
751+
if [[ "$3" == "shell" && "$4" == "getprop" ]]; then
752+
echo "My_AVD_Name"
753+
exit 0
754+
fi
755+
if [[ "$3" == "emu" ]]; then
756+
echo "WRONG_NAME"
757+
echo "OK"
758+
exit 0
759+
fi
760+
exit 1
761+
""");
762+
763+
try {
764+
var runner = new AdbRunner (adbPath);
765+
var name = await runner.GetEmulatorAvdNameAsync ("emulator-5554");
766+
Assert.AreEqual ("My_AVD_Name", name, "Should return getprop result");
767+
} finally {
768+
CleanupFakeAdb (adbPath);
769+
}
770+
}
771+
772+
[Test]
773+
public async Task GetEmulatorAvdNameAsync_FallsBackToEmuAvdName ()
774+
{
775+
// getprop returns empty — should fall back to emu avd name
776+
var adbPath = CreateFakeAdb ("""
777+
if [[ "$3" == "shell" && "$4" == "getprop" ]]; then
778+
echo ""
779+
exit 0
780+
fi
781+
if [[ "$3" == "emu" ]]; then
782+
echo "Fallback_AVD"
783+
echo "OK"
784+
exit 0
785+
fi
786+
exit 1
787+
""");
788+
789+
try {
790+
var runner = new AdbRunner (adbPath);
791+
var name = await runner.GetEmulatorAvdNameAsync ("emulator-5554");
792+
Assert.AreEqual ("Fallback_AVD", name, "Should fall back to emu avd name");
793+
} finally {
794+
CleanupFakeAdb (adbPath);
795+
}
796+
}
797+
798+
[Test]
799+
public async Task GetEmulatorAvdNameAsync_BothFail_ReturnsNull ()
800+
{
801+
// Both getprop and emu avd name return empty
802+
var adbPath = CreateFakeAdb ("""
803+
echo ""
804+
exit 0
805+
""");
806+
807+
try {
808+
var runner = new AdbRunner (adbPath);
809+
var name = await runner.GetEmulatorAvdNameAsync ("emulator-5554");
810+
Assert.IsNull (name, "Should return null when both methods fail");
811+
} finally {
812+
CleanupFakeAdb (adbPath);
813+
}
814+
}
815+
816+
[Test]
817+
public async Task ListDevicesAsync_SkipsAvdQueryForOfflineEmulators ()
818+
{
819+
// adb devices returns one online emulator and one offline emulator.
820+
// Only the online one should get an AVD name query.
821+
var adbPath = CreateFakeAdb ("""
822+
if [[ "$1" == "devices" ]]; then
823+
echo "List of devices attached"
824+
echo "emulator-5554 device product:sdk_gphone64_arm64 model:sdk_gphone64_arm64 device:emu64a transport_id:1"
825+
echo "emulator-5556 offline"
826+
exit 0
827+
fi
828+
if [[ "$1" == "-s" && "$2" == "emulator-5554" && "$3" == "shell" && "$4" == "getprop" ]]; then
829+
echo "Online_AVD"
830+
exit 0
831+
fi
832+
if [[ "$1" == "-s" && "$2" == "emulator-5556" ]]; then
833+
# This should NOT be called for offline emulators.
834+
# Return a name anyway so we can detect if it was incorrectly queried.
835+
echo "OFFLINE_SHOULD_NOT_APPEAR"
836+
exit 0
837+
fi
838+
exit 1
839+
""");
840+
841+
try {
842+
var runner = new AdbRunner (adbPath);
843+
var devices = await runner.ListDevicesAsync ();
844+
845+
Assert.AreEqual (2, devices.Count, "Should return both emulators");
846+
847+
var online = devices.First (d => d.Serial == "emulator-5554");
848+
var offline = devices.First (d => d.Serial == "emulator-5556");
849+
850+
Assert.AreEqual (AdbDeviceStatus.Online, online.Status);
851+
Assert.AreEqual ("Online_AVD", online.AvdName, "Online emulator should have AVD name");
852+
853+
Assert.AreEqual (AdbDeviceStatus.Offline, offline.Status);
854+
Assert.IsNull (offline.AvdName, "Offline emulator should NOT have AVD name queried");
855+
} finally {
856+
CleanupFakeAdb (adbPath);
857+
}
858+
}
717859
}

0 commit comments

Comments
 (0)