From bc5ecce26421809ebb147c4375caf0f3fa989eff Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Wed, 8 Apr 2026 17:40:10 +0100 Subject: [PATCH 1/5] Add ADB device tracking via host:track-devices-l protocol Add AdbDeviceTracker class for real-time device connection monitoring via the ADB daemon socket protocol. Connects to localhost:5037, sends host:track-devices-l, and pushes device list updates through a callback. Features: - Auto-reconnect with exponential backoff (500ms to 16s) - Callback-based StartAsync() with CancellationToken support - CurrentDevices snapshot property - IDisposable lifecycle management - Reuses AdbRunner.ParseAdbDevicesOutput() for parsing Includes 11 unit tests covering protocol parsing, edge cases, and lifecycle management. PublicAPI entries for both net10.0 and netstandard2.0. Closes #323 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 5 + .../netstandard2.0/PublicAPI.Unshipped.txt | 5 + .../Runners/AdbDeviceTracker.cs | 189 ++++++++++++++++++ .../AdbDeviceTrackerTests.cs | 127 ++++++++++++ 4 files changed, 326 insertions(+) create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt index 61fc0e0b..ecab0d25 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -199,3 +199,8 @@ virtual Xamarin.Android.Tools.AdbRunner.ListReversePortsAsync(string! serial, Sy virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Xamarin.Android.Tools.AdbDeviceTracker +Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(string? adbPath = null, int port = 5037, System.Collections.Generic.IDictionary? environmentVariables = null, System.Action? logger = null) -> void +Xamarin.Android.Tools.AdbDeviceTracker.CurrentDevices.get -> System.Collections.Generic.IReadOnlyList! +Xamarin.Android.Tools.AdbDeviceTracker.Dispose() -> void +Xamarin.Android.Tools.AdbDeviceTracker.StartAsync(System.Action!>! onDevicesChanged, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index 61fc0e0b..ecab0d25 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -199,3 +199,8 @@ virtual Xamarin.Android.Tools.AdbRunner.ListReversePortsAsync(string! serial, Sy virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! serial, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! +Xamarin.Android.Tools.AdbDeviceTracker +Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(string? adbPath = null, int port = 5037, System.Collections.Generic.IDictionary? environmentVariables = null, System.Action? logger = null) -> void +Xamarin.Android.Tools.AdbDeviceTracker.CurrentDevices.get -> System.Collections.Generic.IReadOnlyList! +Xamarin.Android.Tools.AdbDeviceTracker.Dispose() -> void +Xamarin.Android.Tools.AdbDeviceTracker.StartAsync(System.Action!>! onDevicesChanged, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs new file mode 100644 index 00000000..1a67b612 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs @@ -0,0 +1,189 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Net.Sockets; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Xamarin.Android.Tools; + +/// +/// Monitors ADB device connections in real-time via the host:track-devices-l socket protocol. +/// Pushes device list updates through a callback whenever devices connect, disconnect, or change state. +/// +public sealed class AdbDeviceTracker : IDisposable +{ + readonly int port; + readonly Action logger; + readonly string? adbPath; + readonly IDictionary? environmentVariables; + IReadOnlyList currentDevices = Array.Empty (); + CancellationTokenSource? trackingCts; + bool disposed; + + /// + /// Creates a new AdbDeviceTracker. + /// + /// Optional path to the adb executable for starting the server if needed. + /// ADB daemon port (default 5037). + /// Optional environment variables for adb processes. + /// Optional logger callback. + public AdbDeviceTracker (string? adbPath = null, int port = 5037, + IDictionary? environmentVariables = null, + Action? logger = null) + { + if (port <= 0 || port > 65535) + throw new ArgumentOutOfRangeException (nameof (port), "Port must be between 1 and 65535."); + this.adbPath = adbPath; + this.port = port; + this.environmentVariables = environmentVariables; + this.logger = logger ?? RunnerDefaults.NullLogger; + } + + /// + /// Current snapshot of tracked devices. + /// + public IReadOnlyList CurrentDevices => currentDevices; + + /// + /// Starts tracking device changes. Calls whenever + /// the device list changes. Blocks until cancelled or disposed. + /// Automatically reconnects on connection drops with exponential backoff. + /// + /// Callback invoked with the updated device list on each change. + /// Token to stop tracking. + public async Task StartAsync ( + Action> onDevicesChanged, + CancellationToken cancellationToken = default) + { + if (onDevicesChanged == null) + throw new ArgumentNullException (nameof (onDevicesChanged)); + if (disposed) + throw new ObjectDisposedException (nameof (AdbDeviceTracker)); + + trackingCts = CancellationTokenSource.CreateLinkedTokenSource (cancellationToken); + var token = trackingCts.Token; + var backoffMs = InitialBackoffMs; + + while (!token.IsCancellationRequested) { + try { + await TrackDevicesAsync (onDevicesChanged, token).ConfigureAwait (false); + } catch (OperationCanceledException) when (token.IsCancellationRequested) { + break; + } catch (Exception ex) { + logger.Invoke (TraceLevel.Warning, $"ADB tracking connection lost: {ex.Message}. Reconnecting in {backoffMs}ms..."); + try { + await Task.Delay (backoffMs, token).ConfigureAwait (false); + } catch (OperationCanceledException) { + break; + } + backoffMs = Math.Min (backoffMs * 2, MaxBackoffMs); + continue; + } + // Reset backoff on clean connection + backoffMs = InitialBackoffMs; + } + } + + const int InitialBackoffMs = 500; + const int MaxBackoffMs = 16000; + + async Task TrackDevicesAsync ( + Action> onDevicesChanged, + CancellationToken cancellationToken) + { + using var client = new TcpClient (); +#if NET5_0_OR_GREATER + await client.ConnectAsync ("127.0.0.1", port, cancellationToken).ConfigureAwait (false); +#else + await client.ConnectAsync ("127.0.0.1", port).ConfigureAwait (false); + cancellationToken.ThrowIfCancellationRequested (); +#endif + + var stream = client.GetStream (); + logger.Invoke (TraceLevel.Verbose, "Connected to ADB daemon, sending track-devices-l command"); + + // Send: <4-digit hex length> + var command = "host:track-devices-l"; + var header = command.Length.ToString ("x4") + command; + var headerBytes = Encoding.ASCII.GetBytes (header); + await stream.WriteAsync (headerBytes, 0, headerBytes.Length, cancellationToken).ConfigureAwait (false); + await stream.FlushAsync (cancellationToken).ConfigureAwait (false); + + // Read response status (OKAY or FAIL) + var status = await ReadExactAsync (stream, 4, cancellationToken).ConfigureAwait (false); + if (status != "OKAY") { + var failMsg = await TryReadLengthPrefixedAsync (stream, cancellationToken).ConfigureAwait (false); + throw new InvalidOperationException ($"ADB daemon rejected track-devices: {status} {failMsg}"); + } + + logger.Invoke (TraceLevel.Verbose, "ADB tracking active"); + + // Read length-prefixed device list updates + while (!cancellationToken.IsCancellationRequested) { + var payload = await TryReadLengthPrefixedAsync (stream, cancellationToken).ConfigureAwait (false); + if (payload == null) + throw new IOException ("ADB daemon closed the connection."); + + var lines = payload.Split ('\n'); + var devices = AdbRunner.ParseAdbDevicesOutput (lines); + currentDevices = devices; + onDevicesChanged (devices); + } + } + + internal static async Task TryReadLengthPrefixedAsync (Stream stream, CancellationToken cancellationToken) + { + // Length is a 4-digit hex string + var lengthHex = await ReadExactOrNullAsync (stream, 4, cancellationToken).ConfigureAwait (false); + if (lengthHex == null) + return null; + + if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) + throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); + + if (length == 0) + return string.Empty; + + return await ReadExactAsync (stream, length, cancellationToken).ConfigureAwait (false); + } + + static async Task ReadExactAsync (Stream stream, int count, CancellationToken cancellationToken) + { + var result = await ReadExactOrNullAsync (stream, count, cancellationToken).ConfigureAwait (false); + return result ?? throw new IOException ($"Unexpected end of stream (expected {count} bytes)."); + } + + static async Task ReadExactOrNullAsync (Stream stream, int count, CancellationToken cancellationToken) + { + var buffer = new byte [count]; + var totalRead = 0; + while (totalRead < count) { + cancellationToken.ThrowIfCancellationRequested (); +#if NET5_0_OR_GREATER + var read = await stream.ReadAsync (buffer.AsMemory (totalRead, count - totalRead), cancellationToken).ConfigureAwait (false); +#else + var read = await stream.ReadAsync (buffer, totalRead, count - totalRead, cancellationToken).ConfigureAwait (false); +#endif + if (read == 0) + return totalRead == 0 ? null : throw new IOException ($"Unexpected end of stream (read {totalRead} of {count} bytes)."); + totalRead += read; + } + return Encoding.ASCII.GetString (buffer, 0, count); + } + + public void Dispose () + { + if (disposed) + return; + disposed = true; + trackingCts?.Cancel (); + trackingCts?.Dispose (); + } +} diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs new file mode 100644 index 00000000..cbed3f6d --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs @@ -0,0 +1,127 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests; + +[TestFixture] +public class AdbDeviceTrackerTests +{ + [Test] + public void Constructor_InvalidPort_ThrowsArgumentOutOfRangeException () + { + Assert.Throws (() => new AdbDeviceTracker (port: 0)); + Assert.Throws (() => new AdbDeviceTracker (port: -1)); + Assert.Throws (() => new AdbDeviceTracker (port: 70000)); + } + + [Test] + public void Constructor_ValidPort_Succeeds () + { + using var tracker = new AdbDeviceTracker (port: 5037); + Assert.IsNotNull (tracker); + Assert.AreEqual (0, tracker.CurrentDevices.Count); + } + + [Test] + public void StartAsync_NullCallback_ThrowsArgumentNullException () + { + using var tracker = new AdbDeviceTracker (); + Assert.ThrowsAsync (() => tracker.StartAsync (null!)); + } + + [Test] + public void StartAsync_AfterDispose_ThrowsObjectDisposedException () + { + var tracker = new AdbDeviceTracker (); + tracker.Dispose (); + Assert.ThrowsAsync (() => tracker.StartAsync (_ => { })); + } + + [Test] + public void Dispose_MultipleTimes_DoesNotThrow () + { + var tracker = new AdbDeviceTracker (); + tracker.Dispose (); + Assert.DoesNotThrow (() => tracker.Dispose ()); + } + + // --- TryReadLengthPrefixedAsync tests --- + + [Test] + public async Task TryReadLengthPrefixedAsync_ValidPayload () + { + var payload = "emulator-5554\tdevice\n"; + var hex = payload.Length.ToString ("x4"); + var data = Encoding.ASCII.GetBytes (hex + payload); + using var stream = new MemoryStream (data); + + var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + Assert.AreEqual (payload, result); + } + + [Test] + public async Task TryReadLengthPrefixedAsync_EmptyPayload () + { + var data = Encoding.ASCII.GetBytes ("0000"); + using var stream = new MemoryStream (data); + + var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + Assert.AreEqual (string.Empty, result); + } + + [Test] + public async Task TryReadLengthPrefixedAsync_EndOfStream_ReturnsNull () + { + using var stream = new MemoryStream (Array.Empty ()); + + var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + Assert.IsNull (result); + } + + [Test] + public async Task TryReadLengthPrefixedAsync_MultipleDevices () + { + var payload = + "0A041FDD400327\tdevice product:redfin model:Pixel_5 device:redfin transport_id:2\n" + + "emulator-5554\tdevice product:sdk_gphone64_x86_64 model:sdk_gphone64_x86_64 device:emu64xa transport_id:1\n"; + var hex = payload.Length.ToString ("x4"); + var data = Encoding.ASCII.GetBytes (hex + payload); + using var stream = new MemoryStream (data); + + var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + Assert.IsNotNull (result); + + var devices = AdbRunner.ParseAdbDevicesOutput (result!.Split ('\n')); + Assert.AreEqual (2, devices.Count); + Assert.AreEqual ("0A041FDD400327", devices [0].Serial); + Assert.AreEqual ("emulator-5554", devices [1].Serial); + } + + [Test] + public void TryReadLengthPrefixedAsync_InvalidHex_ThrowsFormatException () + { + var data = Encoding.ASCII.GetBytes ("ZZZZ"); + using var stream = new MemoryStream (data); + + Assert.ThrowsAsync ( + () => AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None)); + } + + [Test] + public void TryReadLengthPrefixedAsync_TruncatedPayload_ThrowsIOException () + { + // Header says 100 bytes but only 5 are present + var data = Encoding.ASCII.GetBytes ("0064hello"); + using var stream = new MemoryStream (data); + + Assert.ThrowsAsync ( + () => AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None)); + } +} From 5bea4a456a533d91f8896b9985188821c726f416 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 13 Apr 2026 09:18:37 -0500 Subject: [PATCH 2/5] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs index 1a67b612..3e921c8d 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs @@ -5,7 +5,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.Linq; using System.Net.Sockets; using System.Text; using System.Threading; From 7da61a66045ac6c574904ee0b94a1f233865a14c Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 9 Apr 2026 16:00:16 +0100 Subject: [PATCH 3/5] Fix AdbDeviceTracker blockers and extract internal AdbClient Address reviewer feedback: - Extract internal AdbClient class encapsulating the ADB daemon TCP socket protocol (connect, send command, read status, read payloads). Future AdbRunner operations can reuse this instead of shelling out. - AdbClient is byte-oriented at the transport layer with string helpers layered on top for ASCII protocol use cases. - AdbResponseStatus enum for type-safe OKAY/FAIL handling. - AdbDeviceTracker now uses AdbClient via composition. - Reentrancy guard (lock + isTracking flag, throws InvalidOperationException) - Removed unused adbPath/environmentVariables constructor params - Narrowed catch to IOException/SocketException/ObjectDisposedException - Proper disposal: activeClient.Close() unblocks pending reads - CTS cleanup in finally block prevents leaks - Removed unused `using System.Linq` and dead backoff reset Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../PublicAPI/net10.0/PublicAPI.Unshipped.txt | 2 +- .../netstandard2.0/PublicAPI.Unshipped.txt | 2 +- .../Runners/AdbClient.cs | 210 ++++++++++++++++++ .../Runners/AdbDeviceTracker.cs | 175 ++++++--------- .../Runners/AdbResponseStatus.cs | 13 ++ .../AdbDeviceTrackerTests.cs | 44 ++-- 6 files changed, 326 insertions(+), 120 deletions(-) create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs create mode 100644 src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbResponseStatus.cs diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt index ecab0d25..858cfa6c 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt @@ -200,7 +200,7 @@ virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! seria virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Xamarin.Android.Tools.AdbDeviceTracker -Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(string? adbPath = null, int port = 5037, System.Collections.Generic.IDictionary? environmentVariables = null, System.Action? logger = null) -> void +Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(int port = 5037, System.Action? logger = null) -> void Xamarin.Android.Tools.AdbDeviceTracker.CurrentDevices.get -> System.Collections.Generic.IReadOnlyList! Xamarin.Android.Tools.AdbDeviceTracker.Dispose() -> void Xamarin.Android.Tools.AdbDeviceTracker.StartAsync(System.Action!>! onDevicesChanged, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! diff --git a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt index ecab0d25..858cfa6c 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt +++ b/src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt @@ -200,7 +200,7 @@ virtual Xamarin.Android.Tools.AdbRunner.RemoveAllReversePortsAsync(string! seria virtual Xamarin.Android.Tools.AdbRunner.RemoveReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! virtual Xamarin.Android.Tools.AdbRunner.ReversePortAsync(string! serial, Xamarin.Android.Tools.AdbPortSpec! remote, Xamarin.Android.Tools.AdbPortSpec! local, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! Xamarin.Android.Tools.AdbDeviceTracker -Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(string? adbPath = null, int port = 5037, System.Collections.Generic.IDictionary? environmentVariables = null, System.Action? logger = null) -> void +Xamarin.Android.Tools.AdbDeviceTracker.AdbDeviceTracker(int port = 5037, System.Action? logger = null) -> void Xamarin.Android.Tools.AdbDeviceTracker.CurrentDevices.get -> System.Collections.Generic.IReadOnlyList! Xamarin.Android.Tools.AdbDeviceTracker.Dispose() -> void Xamarin.Android.Tools.AdbDeviceTracker.StartAsync(System.Action!>! onDevicesChanged, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task! diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs new file mode 100644 index 00000000..463eb39e --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs @@ -0,0 +1,210 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Net.Sockets; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Xamarin.Android.Tools; + +/// +/// Low-level ADB daemon socket protocol client. +/// Encapsulates a single TCP connection to the ADB server and exposes +/// the wire protocol operations (send command, read status, read length-prefixed payloads). +/// One instance = one connection. Dispose closes the socket. +/// +internal sealed class AdbClient : IDisposable +{ + readonly TcpClient client; + NetworkStream? stream; + bool disposed; + + public AdbClient () + { + client = new TcpClient (); + } + + /// + /// Connects to the ADB daemon at 127.0.0.1 on the specified port. + /// + public async Task ConnectAsync (int port, CancellationToken cancellationToken = default) + { + ThrowIfDisposed (); +#if NET5_0_OR_GREATER + await client.ConnectAsync ("127.0.0.1", port, cancellationToken).ConfigureAwait (false); +#else + await client.ConnectAsync ("127.0.0.1", port).ConfigureAwait (false); + cancellationToken.ThrowIfCancellationRequested (); +#endif + stream = client.GetStream (); + } + + /// + /// Sends a length-prefixed command to the ADB daemon. + /// Wire format: <4-digit hex byte length><command bytes> + /// + public async Task SendCommandAsync (string command, CancellationToken cancellationToken = default) + { + var s = GetStream (); + var commandBytes = Encoding.ASCII.GetBytes (command); + var header = commandBytes.Length.ToString ("x4"); + var headerBytes = Encoding.ASCII.GetBytes (header); + + await s.WriteAsync (headerBytes, 0, headerBytes.Length, cancellationToken).ConfigureAwait (false); + await s.WriteAsync (commandBytes, 0, commandBytes.Length, cancellationToken).ConfigureAwait (false); + await s.FlushAsync (cancellationToken).ConfigureAwait (false); + } + + /// + /// Reads the 4-byte status response from the ADB daemon. + /// Returns or . + /// + public async Task ReadStatusAsync (CancellationToken cancellationToken = default) + { + var s = GetStream (); + var statusBytes = await ReadExactBytesAsync (s, 4, cancellationToken).ConfigureAwait (false); + var status = Encoding.ASCII.GetString (statusBytes, 0, 4); + return status switch { + "OKAY" => AdbResponseStatus.Okay, + "FAIL" => AdbResponseStatus.Fail, + _ => throw new InvalidOperationException ($"Unexpected ADB status: '{status}'"), + }; + } + + /// + /// Reads the failure message after a FAIL status. + /// + public async Task ReadFailMessageAsync (CancellationToken cancellationToken = default) + { + return await ReadLengthPrefixedStringAsync (cancellationToken).ConfigureAwait (false) ?? string.Empty; + } + + /// + /// Ensures the last status was OKAY; throws with the FAIL message otherwise. + /// + public async Task EnsureOkayAsync (CancellationToken cancellationToken = default) + { + var status = await ReadStatusAsync (cancellationToken).ConfigureAwait (false); + if (status == AdbResponseStatus.Fail) { + var message = await ReadFailMessageAsync (cancellationToken).ConfigureAwait (false); + throw new InvalidOperationException ($"ADB command failed: {message}"); + } + } + + /// + /// Reads a length-prefixed ASCII string payload from the daemon. + /// Returns null if the connection is closed cleanly before the length prefix. + /// + public async Task ReadLengthPrefixedStringAsync (CancellationToken cancellationToken = default) + { + var bytes = await ReadLengthPrefixedBytesAsync (cancellationToken).ConfigureAwait (false); + if (bytes == null) + return null; + return Encoding.ASCII.GetString (bytes, 0, bytes.Length); + } + + /// + /// Reads a length-prefixed byte payload from the daemon. + /// Returns null if the connection is closed cleanly before the length prefix. + /// + public async Task ReadLengthPrefixedBytesAsync (CancellationToken cancellationToken = default) + { + var s = GetStream (); + var lengthBytes = await ReadExactBytesOrNullAsync (s, 4, cancellationToken).ConfigureAwait (false); + if (lengthBytes == null) + return null; + + var lengthHex = Encoding.ASCII.GetString (lengthBytes, 0, 4); + if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) + throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); + + if (length == 0) + return Array.Empty (); + + return await ReadExactBytesAsync (s, length, cancellationToken).ConfigureAwait (false); + } + + /// + /// Forcibly closes the underlying socket, unblocking any pending reads. + /// + public void Close () + { + client.Close (); + } + + public void Dispose () + { + if (disposed) + return; + disposed = true; + client.Close (); + client.Dispose (); + } + + NetworkStream GetStream () + { + ThrowIfDisposed (); + if (stream == null) + throw new InvalidOperationException ("Not connected. Call ConnectAsync first."); + return stream; + } + + void ThrowIfDisposed () + { + if (disposed) + throw new ObjectDisposedException (nameof (AdbClient)); + } + + /// + /// Reads a length-prefixed ASCII string from a raw stream. + /// Useful for testing and for callers that already have a stream. + /// + internal static async Task ReadLengthPrefixedStringFromStreamAsync (Stream stream, CancellationToken cancellationToken) + { + var lengthBytes = await ReadExactBytesOrNullAsync (stream, 4, cancellationToken).ConfigureAwait (false); + if (lengthBytes == null) + return null; + + var lengthHex = Encoding.ASCII.GetString (lengthBytes, 0, 4); + if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) + throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); + + if (length == 0) + return string.Empty; + + var payload = await ReadExactBytesAsync (stream, length, cancellationToken).ConfigureAwait (false); + return Encoding.ASCII.GetString (payload, 0, payload.Length); + } + + static async Task ReadExactBytesAsync (Stream stream, int count, CancellationToken cancellationToken) + { + var result = await ReadExactBytesOrNullAsync (stream, count, cancellationToken).ConfigureAwait (false); + if (result == null) + throw new IOException ($"Unexpected end of stream (expected {count} bytes)."); + return result; + } + + static async Task ReadExactBytesOrNullAsync (Stream stream, int count, CancellationToken cancellationToken) + { + var buffer = new byte [count]; + var totalRead = 0; + while (totalRead < count) { + cancellationToken.ThrowIfCancellationRequested (); +#if NET5_0_OR_GREATER + var read = await stream.ReadAsync (buffer.AsMemory (totalRead, count - totalRead), cancellationToken).ConfigureAwait (false); +#else + var read = await stream.ReadAsync (buffer, totalRead, count - totalRead, cancellationToken).ConfigureAwait (false); +#endif + if (read == 0) { + if (totalRead == 0) + return null; + throw new IOException ($"Unexpected end of stream (read {totalRead} of {count} bytes)."); + } + totalRead += read; + } + return buffer; + } +} diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs index 1a67b612..a756e121 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs @@ -5,9 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; -using System.Linq; using System.Net.Sockets; -using System.Text; using System.Threading; using System.Threading.Tasks; @@ -19,30 +17,26 @@ namespace Xamarin.Android.Tools; /// public sealed class AdbDeviceTracker : IDisposable { + readonly object syncLock = new object (); readonly int port; readonly Action logger; - readonly string? adbPath; - readonly IDictionary? environmentVariables; - IReadOnlyList currentDevices = Array.Empty (); + volatile IReadOnlyList currentDevices = Array.Empty (); + AdbClient? activeClient; CancellationTokenSource? trackingCts; + bool isTracking; bool disposed; /// /// Creates a new AdbDeviceTracker. /// - /// Optional path to the adb executable for starting the server if needed. /// ADB daemon port (default 5037). - /// Optional environment variables for adb processes. /// Optional logger callback. - public AdbDeviceTracker (string? adbPath = null, int port = 5037, - IDictionary? environmentVariables = null, + public AdbDeviceTracker (int port = 5037, Action? logger = null) { if (port <= 0 || port > 65535) throw new ArgumentOutOfRangeException (nameof (port), "Port must be between 1 and 65535."); - this.adbPath = adbPath; this.port = port; - this.environmentVariables = environmentVariables; this.logger = logger ?? RunnerDefaults.NullLogger; } @@ -58,36 +52,52 @@ public AdbDeviceTracker (string? adbPath = null, int port = 5037, /// /// Callback invoked with the updated device list on each change. /// Token to stop tracking. + /// Thrown if tracking is already active. public async Task StartAsync ( Action> onDevicesChanged, CancellationToken cancellationToken = default) { if (onDevicesChanged == null) throw new ArgumentNullException (nameof (onDevicesChanged)); - if (disposed) - throw new ObjectDisposedException (nameof (AdbDeviceTracker)); - trackingCts = CancellationTokenSource.CreateLinkedTokenSource (cancellationToken); - var token = trackingCts.Token; + CancellationTokenSource cts; + lock (syncLock) { + if (disposed) + throw new ObjectDisposedException (nameof (AdbDeviceTracker)); + if (isTracking) + throw new InvalidOperationException ("Tracking is already active. Cancel the token or dispose before starting again."); + isTracking = true; + cts = CancellationTokenSource.CreateLinkedTokenSource (cancellationToken); + trackingCts = cts; + } + + var token = cts.Token; var backoffMs = InitialBackoffMs; - while (!token.IsCancellationRequested) { - try { - await TrackDevicesAsync (onDevicesChanged, token).ConfigureAwait (false); - } catch (OperationCanceledException) when (token.IsCancellationRequested) { - break; - } catch (Exception ex) { - logger.Invoke (TraceLevel.Warning, $"ADB tracking connection lost: {ex.Message}. Reconnecting in {backoffMs}ms..."); + try { + while (!token.IsCancellationRequested) { try { - await Task.Delay (backoffMs, token).ConfigureAwait (false); - } catch (OperationCanceledException) { + await TrackDevicesAsync (onDevicesChanged, token).ConfigureAwait (false); + } catch (OperationCanceledException) when (token.IsCancellationRequested) { break; + } catch (Exception ex) when (ex is IOException || ex is SocketException || ex is ObjectDisposedException) { + if (token.IsCancellationRequested) + break; + logger.Invoke (TraceLevel.Warning, $"ADB tracking connection lost: {ex.Message}. Reconnecting in {backoffMs}ms..."); + try { + await Task.Delay (backoffMs, token).ConfigureAwait (false); + } catch (OperationCanceledException) { + break; + } + backoffMs = Math.Min (backoffMs * 2, MaxBackoffMs); } - backoffMs = Math.Min (backoffMs * 2, MaxBackoffMs); - continue; } - // Reset backoff on clean connection - backoffMs = InitialBackoffMs; + } finally { + lock (syncLock) { + isTracking = false; + trackingCts = null; + } + cts.Dispose (); } } @@ -98,92 +108,47 @@ async Task TrackDevicesAsync ( Action> onDevicesChanged, CancellationToken cancellationToken) { - using var client = new TcpClient (); -#if NET5_0_OR_GREATER - await client.ConnectAsync ("127.0.0.1", port, cancellationToken).ConfigureAwait (false); -#else - await client.ConnectAsync ("127.0.0.1", port).ConfigureAwait (false); - cancellationToken.ThrowIfCancellationRequested (); -#endif - - var stream = client.GetStream (); - logger.Invoke (TraceLevel.Verbose, "Connected to ADB daemon, sending track-devices-l command"); - - // Send: <4-digit hex length> - var command = "host:track-devices-l"; - var header = command.Length.ToString ("x4") + command; - var headerBytes = Encoding.ASCII.GetBytes (header); - await stream.WriteAsync (headerBytes, 0, headerBytes.Length, cancellationToken).ConfigureAwait (false); - await stream.FlushAsync (cancellationToken).ConfigureAwait (false); - - // Read response status (OKAY or FAIL) - var status = await ReadExactAsync (stream, 4, cancellationToken).ConfigureAwait (false); - if (status != "OKAY") { - var failMsg = await TryReadLengthPrefixedAsync (stream, cancellationToken).ConfigureAwait (false); - throw new InvalidOperationException ($"ADB daemon rejected track-devices: {status} {failMsg}"); + var adb = new AdbClient (); + lock (syncLock) { + activeClient = adb; } + try { + await adb.ConnectAsync (port, cancellationToken).ConfigureAwait (false); + logger.Invoke (TraceLevel.Verbose, "Connected to ADB daemon, sending track-devices-l command"); - logger.Invoke (TraceLevel.Verbose, "ADB tracking active"); - - // Read length-prefixed device list updates - while (!cancellationToken.IsCancellationRequested) { - var payload = await TryReadLengthPrefixedAsync (stream, cancellationToken).ConfigureAwait (false); - if (payload == null) - throw new IOException ("ADB daemon closed the connection."); - - var lines = payload.Split ('\n'); - var devices = AdbRunner.ParseAdbDevicesOutput (lines); - currentDevices = devices; - onDevicesChanged (devices); - } - } - - internal static async Task TryReadLengthPrefixedAsync (Stream stream, CancellationToken cancellationToken) - { - // Length is a 4-digit hex string - var lengthHex = await ReadExactOrNullAsync (stream, 4, cancellationToken).ConfigureAwait (false); - if (lengthHex == null) - return null; - - if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) - throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); - - if (length == 0) - return string.Empty; + await adb.SendCommandAsync ("host:track-devices-l", cancellationToken).ConfigureAwait (false); + await adb.EnsureOkayAsync (cancellationToken).ConfigureAwait (false); - return await ReadExactAsync (stream, length, cancellationToken).ConfigureAwait (false); - } + logger.Invoke (TraceLevel.Verbose, "ADB tracking active"); - static async Task ReadExactAsync (Stream stream, int count, CancellationToken cancellationToken) - { - var result = await ReadExactOrNullAsync (stream, count, cancellationToken).ConfigureAwait (false); - return result ?? throw new IOException ($"Unexpected end of stream (expected {count} bytes)."); - } + // Read length-prefixed device list updates + while (!cancellationToken.IsCancellationRequested) { + var payload = await adb.ReadLengthPrefixedStringAsync (cancellationToken).ConfigureAwait (false); + if (payload == null) + throw new IOException ("ADB daemon closed the connection."); - static async Task ReadExactOrNullAsync (Stream stream, int count, CancellationToken cancellationToken) - { - var buffer = new byte [count]; - var totalRead = 0; - while (totalRead < count) { - cancellationToken.ThrowIfCancellationRequested (); -#if NET5_0_OR_GREATER - var read = await stream.ReadAsync (buffer.AsMemory (totalRead, count - totalRead), cancellationToken).ConfigureAwait (false); -#else - var read = await stream.ReadAsync (buffer, totalRead, count - totalRead, cancellationToken).ConfigureAwait (false); -#endif - if (read == 0) - return totalRead == 0 ? null : throw new IOException ($"Unexpected end of stream (read {totalRead} of {count} bytes)."); - totalRead += read; + var lines = payload.Split ('\n'); + var devices = AdbRunner.ParseAdbDevicesOutput (lines); + currentDevices = devices; + onDevicesChanged (devices); + } + } finally { + lock (syncLock) { + activeClient = null; + } + adb.Dispose (); } - return Encoding.ASCII.GetString (buffer, 0, count); } public void Dispose () { - if (disposed) - return; - disposed = true; - trackingCts?.Cancel (); - trackingCts?.Dispose (); + lock (syncLock) { + if (disposed) + return; + disposed = true; + trackingCts?.Cancel (); + activeClient?.Close (); + trackingCts?.Dispose (); + } } } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbResponseStatus.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbResponseStatus.cs new file mode 100644 index 00000000..409eec99 --- /dev/null +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbResponseStatus.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Xamarin.Android.Tools; + +/// +/// Status response from the ADB daemon after sending a command. +/// +internal enum AdbResponseStatus +{ + Okay, + Fail, +} diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs index cbed3f6d..311bc077 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs @@ -44,6 +44,24 @@ public void StartAsync_AfterDispose_ThrowsObjectDisposedException () Assert.ThrowsAsync (() => tracker.StartAsync (_ => { })); } + [Test] + public async Task StartAsync_CalledTwice_ThrowsInvalidOperationException () + { + // Use a port where nothing is listening so ConnectAsync yields quickly + using var tracker = new AdbDeviceTracker (port: 59999); + using var cts = new CancellationTokenSource (); + + // First call sets isTracking synchronously before the first await + var trackingTask = tracker.StartAsync (_ => { }, cts.Token); + + // Second call should throw because tracking is already active + Assert.ThrowsAsync ( + () => tracker.StartAsync (_ => { }, cts.Token)); + + cts.Cancel (); + try { await trackingTask.ConfigureAwait (false); } catch (OperationCanceledException) { } + } + [Test] public void Dispose_MultipleTimes_DoesNotThrow () { @@ -52,41 +70,41 @@ public void Dispose_MultipleTimes_DoesNotThrow () Assert.DoesNotThrow (() => tracker.Dispose ()); } - // --- TryReadLengthPrefixedAsync tests --- + // --- AdbClient protocol tests --- [Test] - public async Task TryReadLengthPrefixedAsync_ValidPayload () + public async Task ReadLengthPrefixedStringFromStreamAsync_ValidPayload () { var payload = "emulator-5554\tdevice\n"; var hex = payload.Length.ToString ("x4"); var data = Encoding.ASCII.GetBytes (hex + payload); using var stream = new MemoryStream (data); - var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); Assert.AreEqual (payload, result); } [Test] - public async Task TryReadLengthPrefixedAsync_EmptyPayload () + public async Task ReadLengthPrefixedStringFromStreamAsync_EmptyPayload () { var data = Encoding.ASCII.GetBytes ("0000"); using var stream = new MemoryStream (data); - var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); Assert.AreEqual (string.Empty, result); } [Test] - public async Task TryReadLengthPrefixedAsync_EndOfStream_ReturnsNull () + public async Task ReadLengthPrefixedStringFromStreamAsync_EndOfStream_ReturnsNull () { using var stream = new MemoryStream (Array.Empty ()); - var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); Assert.IsNull (result); } [Test] - public async Task TryReadLengthPrefixedAsync_MultipleDevices () + public async Task ReadLengthPrefixedStringFromStreamAsync_MultipleDevices () { var payload = "0A041FDD400327\tdevice product:redfin model:Pixel_5 device:redfin transport_id:2\n" + @@ -95,7 +113,7 @@ public async Task TryReadLengthPrefixedAsync_MultipleDevices () var data = Encoding.ASCII.GetBytes (hex + payload); using var stream = new MemoryStream (data); - var result = await AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None); + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); Assert.IsNotNull (result); var devices = AdbRunner.ParseAdbDevicesOutput (result!.Split ('\n')); @@ -105,23 +123,23 @@ public async Task TryReadLengthPrefixedAsync_MultipleDevices () } [Test] - public void TryReadLengthPrefixedAsync_InvalidHex_ThrowsFormatException () + public void ReadLengthPrefixedStringFromStreamAsync_InvalidHex_ThrowsFormatException () { var data = Encoding.ASCII.GetBytes ("ZZZZ"); using var stream = new MemoryStream (data); Assert.ThrowsAsync ( - () => AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None)); + () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); } [Test] - public void TryReadLengthPrefixedAsync_TruncatedPayload_ThrowsIOException () + public void ReadLengthPrefixedStringFromStreamAsync_TruncatedPayload_ThrowsIOException () { // Header says 100 bytes but only 5 are present var data = Encoding.ASCII.GetBytes ("0064hello"); using var stream = new MemoryStream (data); Assert.ThrowsAsync ( - () => AdbDeviceTracker.TryReadLengthPrefixedAsync (stream, CancellationToken.None)); + () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); } } From a8a7f5c648dd17155d5a55110d98ef6603d7cba6 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 30 Apr 2026 16:07:07 +0100 Subject: [PATCH 4/5] Address review feedback: reuse AdbClient, reduce allocations, split tests - AdbClient: add ReconnectAsync() for connection reuse across reconnects - AdbClient: single-buffer SendCommandAsync (one write instead of two) - AdbClient: byte-level status comparison avoids string allocation - AdbClient: deduplicate static/instance length-prefixed read via shared core - AdbDeviceTracker: reuse single AdbClient instance for its lifetime - AdbDeviceTracker: reset backoff after successful connection - Move protocol tests to dedicated AdbClientTests.cs (one type per file) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runners/AdbClient.cs | 118 +++++++++++------- .../Runners/AdbDeviceTracker.cs | 47 +++---- .../AdbClientTests.cs | 86 +++++++++++++ .../AdbDeviceTrackerTests.cs | 75 ----------- 4 files changed, 178 insertions(+), 148 deletions(-) create mode 100644 tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbClientTests.cs diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs index 463eb39e..9be0ea6f 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs @@ -14,32 +14,39 @@ namespace Xamarin.Android.Tools; /// Low-level ADB daemon socket protocol client. /// Encapsulates a single TCP connection to the ADB server and exposes /// the wire protocol operations (send command, read status, read length-prefixed payloads). -/// One instance = one connection. Dispose closes the socket. +/// One instance can be reused across reconnections via . +/// Dispose closes the socket. /// internal sealed class AdbClient : IDisposable { - readonly TcpClient client; + TcpClient? client; NetworkStream? stream; bool disposed; - public AdbClient () - { - client = new TcpClient (); - } - /// /// Connects to the ADB daemon at 127.0.0.1 on the specified port. /// public async Task ConnectAsync (int port, CancellationToken cancellationToken = default) { ThrowIfDisposed (); + var tcp = new TcpClient (); + client = tcp; #if NET5_0_OR_GREATER - await client.ConnectAsync ("127.0.0.1", port, cancellationToken).ConfigureAwait (false); + await tcp.ConnectAsync ("127.0.0.1", port, cancellationToken).ConfigureAwait (false); #else - await client.ConnectAsync ("127.0.0.1", port).ConfigureAwait (false); + await tcp.ConnectAsync ("127.0.0.1", port).ConfigureAwait (false); cancellationToken.ThrowIfCancellationRequested (); #endif - stream = client.GetStream (); + stream = tcp.GetStream (); + } + + /// + /// Closes the current connection and establishes a new one. + /// + public async Task ReconnectAsync (int port, CancellationToken cancellationToken = default) + { + CloseConnection (); + await ConnectAsync (port, cancellationToken).ConfigureAwait (false); } /// @@ -49,29 +56,37 @@ public async Task ConnectAsync (int port, CancellationToken cancellationToken = public async Task SendCommandAsync (string command, CancellationToken cancellationToken = default) { var s = GetStream (); + // Combine length prefix + command into a single write to minimize syscalls var commandBytes = Encoding.ASCII.GetBytes (command); - var header = commandBytes.Length.ToString ("x4"); - var headerBytes = Encoding.ASCII.GetBytes (header); - - await s.WriteAsync (headerBytes, 0, headerBytes.Length, cancellationToken).ConfigureAwait (false); - await s.WriteAsync (commandBytes, 0, commandBytes.Length, cancellationToken).ConfigureAwait (false); + var lengthPrefix = commandBytes.Length.ToString ("x4"); + var packet = new byte [4 + commandBytes.Length]; + Encoding.ASCII.GetBytes (lengthPrefix, 0, 4, packet, 0); + Buffer.BlockCopy (commandBytes, 0, packet, 4, commandBytes.Length); +#if NET5_0_OR_GREATER + await s.WriteAsync (packet.AsMemory (), cancellationToken).ConfigureAwait (false); +#else + await s.WriteAsync (packet, 0, packet.Length, cancellationToken).ConfigureAwait (false); +#endif await s.FlushAsync (cancellationToken).ConfigureAwait (false); } /// /// Reads the 4-byte status response from the ADB daemon. - /// Returns or . /// public async Task ReadStatusAsync (CancellationToken cancellationToken = default) { var s = GetStream (); - var statusBytes = await ReadExactBytesAsync (s, 4, cancellationToken).ConfigureAwait (false); + var statusBytes = await ReadExactBytesIntoNewArrayAsync (s, 4, cancellationToken).ConfigureAwait (false); + // Status is always 4 ASCII chars + if (statusBytes [0] == (byte) 'O' && statusBytes [1] == (byte) 'K' && + statusBytes [2] == (byte) 'A' && statusBytes [3] == (byte) 'Y') + return AdbResponseStatus.Okay; + if (statusBytes [0] == (byte) 'F' && statusBytes [1] == (byte) 'A' && + statusBytes [2] == (byte) 'I' && statusBytes [3] == (byte) 'L') + return AdbResponseStatus.Fail; + var status = Encoding.ASCII.GetString (statusBytes, 0, 4); - return status switch { - "OKAY" => AdbResponseStatus.Okay, - "FAIL" => AdbResponseStatus.Fail, - _ => throw new InvalidOperationException ($"Unexpected ADB status: '{status}'"), - }; + throw new InvalidOperationException ($"Unexpected ADB status: '{status}'"); } /// @@ -100,10 +115,8 @@ public async Task EnsureOkayAsync (CancellationToken cancellationToken = default /// public async Task ReadLengthPrefixedStringAsync (CancellationToken cancellationToken = default) { - var bytes = await ReadLengthPrefixedBytesAsync (cancellationToken).ConfigureAwait (false); - if (bytes == null) - return null; - return Encoding.ASCII.GetString (bytes, 0, bytes.Length); + var s = GetStream (); + return await ReadLengthPrefixedStringCoreAsync (s, cancellationToken).ConfigureAwait (false); } /// @@ -113,18 +126,7 @@ public async Task EnsureOkayAsync (CancellationToken cancellationToken = default public async Task ReadLengthPrefixedBytesAsync (CancellationToken cancellationToken = default) { var s = GetStream (); - var lengthBytes = await ReadExactBytesOrNullAsync (s, 4, cancellationToken).ConfigureAwait (false); - if (lengthBytes == null) - return null; - - var lengthHex = Encoding.ASCII.GetString (lengthBytes, 0, 4); - if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) - throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); - - if (length == 0) - return Array.Empty (); - - return await ReadExactBytesAsync (s, length, cancellationToken).ConfigureAwait (false); + return await ReadLengthPrefixedBytesCoreAsync (s, cancellationToken).ConfigureAwait (false); } /// @@ -132,7 +134,7 @@ public async Task EnsureOkayAsync (CancellationToken cancellationToken = default /// public void Close () { - client.Close (); + CloseConnection (); } public void Dispose () @@ -140,8 +142,18 @@ public void Dispose () if (disposed) return; disposed = true; - client.Close (); - client.Dispose (); + CloseConnection (); + } + + void CloseConnection () + { + stream = null; + var tcp = client; + client = null; + if (tcp != null) { + tcp.Close (); + tcp.Dispose (); + } } NetworkStream GetStream () @@ -158,11 +170,26 @@ void ThrowIfDisposed () throw new ObjectDisposedException (nameof (AdbClient)); } + // --- Shared core implementations (used by both instance and static methods) --- + /// /// Reads a length-prefixed ASCII string from a raw stream. - /// Useful for testing and for callers that already have a stream. + /// Shared core for both instance and static access patterns. /// internal static async Task ReadLengthPrefixedStringFromStreamAsync (Stream stream, CancellationToken cancellationToken) + { + return await ReadLengthPrefixedStringCoreAsync (stream, cancellationToken).ConfigureAwait (false); + } + + static async Task ReadLengthPrefixedStringCoreAsync (Stream stream, CancellationToken cancellationToken) + { + var bytes = await ReadLengthPrefixedBytesCoreAsync (stream, cancellationToken).ConfigureAwait (false); + if (bytes == null) + return null; + return Encoding.ASCII.GetString (bytes, 0, bytes.Length); + } + + static async Task ReadLengthPrefixedBytesCoreAsync (Stream stream, CancellationToken cancellationToken) { var lengthBytes = await ReadExactBytesOrNullAsync (stream, 4, cancellationToken).ConfigureAwait (false); if (lengthBytes == null) @@ -173,13 +200,12 @@ void ThrowIfDisposed () throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); if (length == 0) - return string.Empty; + return Array.Empty (); - var payload = await ReadExactBytesAsync (stream, length, cancellationToken).ConfigureAwait (false); - return Encoding.ASCII.GetString (payload, 0, payload.Length); + return await ReadExactBytesIntoNewArrayAsync (stream, length, cancellationToken).ConfigureAwait (false); } - static async Task ReadExactBytesAsync (Stream stream, int count, CancellationToken cancellationToken) + static async Task ReadExactBytesIntoNewArrayAsync (Stream stream, int count, CancellationToken cancellationToken) { var result = await ReadExactBytesOrNullAsync (stream, count, cancellationToken).ConfigureAwait (false); if (result == null) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs index a756e121..d6f25c38 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbDeviceTracker.cs @@ -20,8 +20,8 @@ public sealed class AdbDeviceTracker : IDisposable readonly object syncLock = new object (); readonly int port; readonly Action logger; + readonly AdbClient adbClient; volatile IReadOnlyList currentDevices = Array.Empty (); - AdbClient? activeClient; CancellationTokenSource? trackingCts; bool isTracking; bool disposed; @@ -38,6 +38,7 @@ public AdbDeviceTracker (int port = 5037, throw new ArgumentOutOfRangeException (nameof (port), "Port must be between 1 and 65535."); this.port = port; this.logger = logger ?? RunnerDefaults.NullLogger; + this.adbClient = new AdbClient (); } /// @@ -90,7 +91,9 @@ public async Task StartAsync ( break; } backoffMs = Math.Min (backoffMs * 2, MaxBackoffMs); + continue; } + backoffMs = InitialBackoffMs; } } finally { lock (syncLock) { @@ -108,35 +111,24 @@ async Task TrackDevicesAsync ( Action> onDevicesChanged, CancellationToken cancellationToken) { - var adb = new AdbClient (); - lock (syncLock) { - activeClient = adb; - } - try { - await adb.ConnectAsync (port, cancellationToken).ConfigureAwait (false); - logger.Invoke (TraceLevel.Verbose, "Connected to ADB daemon, sending track-devices-l command"); + await adbClient.ReconnectAsync (port, cancellationToken).ConfigureAwait (false); + logger.Invoke (TraceLevel.Verbose, "Connected to ADB daemon, sending track-devices-l command"); - await adb.SendCommandAsync ("host:track-devices-l", cancellationToken).ConfigureAwait (false); - await adb.EnsureOkayAsync (cancellationToken).ConfigureAwait (false); + await adbClient.SendCommandAsync ("host:track-devices-l", cancellationToken).ConfigureAwait (false); + await adbClient.EnsureOkayAsync (cancellationToken).ConfigureAwait (false); - logger.Invoke (TraceLevel.Verbose, "ADB tracking active"); + logger.Invoke (TraceLevel.Verbose, "ADB tracking active"); - // Read length-prefixed device list updates - while (!cancellationToken.IsCancellationRequested) { - var payload = await adb.ReadLengthPrefixedStringAsync (cancellationToken).ConfigureAwait (false); - if (payload == null) - throw new IOException ("ADB daemon closed the connection."); + // Read length-prefixed device list updates + while (!cancellationToken.IsCancellationRequested) { + var payload = await adbClient.ReadLengthPrefixedStringAsync (cancellationToken).ConfigureAwait (false); + if (payload == null) + throw new IOException ("ADB daemon closed the connection."); - var lines = payload.Split ('\n'); - var devices = AdbRunner.ParseAdbDevicesOutput (lines); - currentDevices = devices; - onDevicesChanged (devices); - } - } finally { - lock (syncLock) { - activeClient = null; - } - adb.Dispose (); + var lines = payload.Split ('\n'); + var devices = AdbRunner.ParseAdbDevicesOutput (lines); + currentDevices = devices; + onDevicesChanged (devices); } } @@ -147,8 +139,9 @@ public void Dispose () return; disposed = true; trackingCts?.Cancel (); - activeClient?.Close (); + adbClient.Close (); trackingCts?.Dispose (); } + adbClient.Dispose (); } } diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbClientTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbClientTests.cs new file mode 100644 index 00000000..32b434c2 --- /dev/null +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbClientTests.cs @@ -0,0 +1,86 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; + +namespace Xamarin.Android.Tools.Tests; + +[TestFixture] +public class AdbClientTests +{ + [Test] + public async Task ReadLengthPrefixedStringFromStreamAsync_ValidPayload () + { + var payload = "emulator-5554\tdevice\n"; + var hex = payload.Length.ToString ("x4"); + var data = Encoding.ASCII.GetBytes (hex + payload); + using var stream = new MemoryStream (data); + + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); + Assert.AreEqual (payload, result); + } + + [Test] + public async Task ReadLengthPrefixedStringFromStreamAsync_EmptyPayload () + { + var data = Encoding.ASCII.GetBytes ("0000"); + using var stream = new MemoryStream (data); + + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); + Assert.AreEqual (string.Empty, result); + } + + [Test] + public async Task ReadLengthPrefixedStringFromStreamAsync_EndOfStream_ReturnsNull () + { + using var stream = new MemoryStream (Array.Empty ()); + + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); + Assert.IsNull (result); + } + + [Test] + public async Task ReadLengthPrefixedStringFromStreamAsync_MultipleDevices () + { + var payload = + "0A041FDD400327\tdevice product:redfin model:Pixel_5 device:redfin transport_id:2\n" + + "emulator-5554\tdevice product:sdk_gphone64_x86_64 model:sdk_gphone64_x86_64 device:emu64xa transport_id:1\n"; + var hex = payload.Length.ToString ("x4"); + var data = Encoding.ASCII.GetBytes (hex + payload); + using var stream = new MemoryStream (data); + + var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); + Assert.IsNotNull (result); + + var devices = AdbRunner.ParseAdbDevicesOutput (result!.Split ('\n')); + Assert.AreEqual (2, devices.Count); + Assert.AreEqual ("0A041FDD400327", devices [0].Serial); + Assert.AreEqual ("emulator-5554", devices [1].Serial); + } + + [Test] + public void ReadLengthPrefixedStringFromStreamAsync_InvalidHex_ThrowsFormatException () + { + var data = Encoding.ASCII.GetBytes ("ZZZZ"); + using var stream = new MemoryStream (data); + + Assert.ThrowsAsync ( + () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); + } + + [Test] + public void ReadLengthPrefixedStringFromStreamAsync_TruncatedPayload_ThrowsIOException () + { + // Header says 100 bytes but only 5 are present + var data = Encoding.ASCII.GetBytes ("0064hello"); + using var stream = new MemoryStream (data); + + Assert.ThrowsAsync ( + () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); + } +} diff --git a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs index 311bc077..2670cded 100644 --- a/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs +++ b/tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbDeviceTrackerTests.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.IO; -using System.Text; using System.Threading; using System.Threading.Tasks; using NUnit.Framework; @@ -69,77 +67,4 @@ public void Dispose_MultipleTimes_DoesNotThrow () tracker.Dispose (); Assert.DoesNotThrow (() => tracker.Dispose ()); } - - // --- AdbClient protocol tests --- - - [Test] - public async Task ReadLengthPrefixedStringFromStreamAsync_ValidPayload () - { - var payload = "emulator-5554\tdevice\n"; - var hex = payload.Length.ToString ("x4"); - var data = Encoding.ASCII.GetBytes (hex + payload); - using var stream = new MemoryStream (data); - - var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); - Assert.AreEqual (payload, result); - } - - [Test] - public async Task ReadLengthPrefixedStringFromStreamAsync_EmptyPayload () - { - var data = Encoding.ASCII.GetBytes ("0000"); - using var stream = new MemoryStream (data); - - var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); - Assert.AreEqual (string.Empty, result); - } - - [Test] - public async Task ReadLengthPrefixedStringFromStreamAsync_EndOfStream_ReturnsNull () - { - using var stream = new MemoryStream (Array.Empty ()); - - var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); - Assert.IsNull (result); - } - - [Test] - public async Task ReadLengthPrefixedStringFromStreamAsync_MultipleDevices () - { - var payload = - "0A041FDD400327\tdevice product:redfin model:Pixel_5 device:redfin transport_id:2\n" + - "emulator-5554\tdevice product:sdk_gphone64_x86_64 model:sdk_gphone64_x86_64 device:emu64xa transport_id:1\n"; - var hex = payload.Length.ToString ("x4"); - var data = Encoding.ASCII.GetBytes (hex + payload); - using var stream = new MemoryStream (data); - - var result = await AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None); - Assert.IsNotNull (result); - - var devices = AdbRunner.ParseAdbDevicesOutput (result!.Split ('\n')); - Assert.AreEqual (2, devices.Count); - Assert.AreEqual ("0A041FDD400327", devices [0].Serial); - Assert.AreEqual ("emulator-5554", devices [1].Serial); - } - - [Test] - public void ReadLengthPrefixedStringFromStreamAsync_InvalidHex_ThrowsFormatException () - { - var data = Encoding.ASCII.GetBytes ("ZZZZ"); - using var stream = new MemoryStream (data); - - Assert.ThrowsAsync ( - () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); - } - - [Test] - public void ReadLengthPrefixedStringFromStreamAsync_TruncatedPayload_ThrowsIOException () - { - // Header says 100 bytes but only 5 are present - var data = Encoding.ASCII.GetBytes ("0064hello"); - using var stream = new MemoryStream (data); - - Assert.ThrowsAsync ( - () => AdbClient.ReadLengthPrefixedStringFromStreamAsync (stream, CancellationToken.None)); - } } From 93233c1bcb45519610aacf38afccbc294ff7e8f0 Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 30 Apr 2026 16:40:53 +0100 Subject: [PATCH 5/5] Minimize byte[] allocations in AdbClient using ArrayPool and buffer reuse - Add reusable 4-byte headerBuffer field for status/length reads (zero alloc per call) - SendCommandAsync: use ArrayPool.Shared for packet buffer, encode command directly without intermediate byte[] (eliminates 2 allocations) - ReadLengthPrefixedStringAsync: rent payload buffer from ArrayPool, return after decode - Add WriteHexLength/ParseHexLength helpers: emit/parse 4-digit hex without string alloc - ReadStatusAsync: reads into headerBuffer instead of allocating new byte[4] - Split I/O into ReadExactBytesIntoBufferAsync and TryReadExactBytesIntoBufferAsync - Static test method keeps simple allocations (no instance state available) - Document non-thread-safe invariant in class remarks Techniques modeled after DownloadUtils.cs (ArrayPool rent/return) in this repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Runners/AdbClient.cs | 181 +++++++++++++----- 1 file changed, 133 insertions(+), 48 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs index 9be0ea6f..96833732 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbClient.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Buffers; using System.IO; using System.Net.Sockets; using System.Text; @@ -17,8 +18,14 @@ namespace Xamarin.Android.Tools; /// One instance can be reused across reconnections via . /// Dispose closes the socket. /// +/// +/// This class is not thread-safe. All protocol operations must be serialized by the caller. +/// internal sealed class AdbClient : IDisposable { + // Reusable 4-byte buffer for status/length reads (safe: single-caller, non-concurrent) + readonly byte[] headerBuffer = new byte [4]; + TcpClient? client; NetworkStream? stream; bool disposed; @@ -56,17 +63,24 @@ public async Task ReconnectAsync (int port, CancellationToken cancellationToken public async Task SendCommandAsync (string command, CancellationToken cancellationToken = default) { var s = GetStream (); - // Combine length prefix + command into a single write to minimize syscalls - var commandBytes = Encoding.ASCII.GetBytes (command); - var lengthPrefix = commandBytes.Length.ToString ("x4"); - var packet = new byte [4 + commandBytes.Length]; - Encoding.ASCII.GetBytes (lengthPrefix, 0, 4, packet, 0); - Buffer.BlockCopy (commandBytes, 0, packet, 4, commandBytes.Length); + // Compute byte count without allocating a separate commandBytes array + var byteCount = Encoding.ASCII.GetByteCount (command); + var packetLength = 4 + byteCount; + var packet = ArrayPool.Shared.Rent (packetLength); + try { + // Write 4-hex-digit length prefix directly into packet + WriteHexLength (packet, byteCount); + // Encode command directly into packet after the prefix + Encoding.ASCII.GetBytes (command, 0, command.Length, packet, 4); #if NET5_0_OR_GREATER - await s.WriteAsync (packet.AsMemory (), cancellationToken).ConfigureAwait (false); + await s.WriteAsync (packet.AsMemory (0, packetLength), cancellationToken).ConfigureAwait (false); #else - await s.WriteAsync (packet, 0, packet.Length, cancellationToken).ConfigureAwait (false); + await s.WriteAsync (packet, 0, packetLength, cancellationToken).ConfigureAwait (false); #endif + } + finally { + ArrayPool.Shared.Return (packet); + } await s.FlushAsync (cancellationToken).ConfigureAwait (false); } @@ -76,16 +90,15 @@ public async Task SendCommandAsync (string command, CancellationToken cancellati public async Task ReadStatusAsync (CancellationToken cancellationToken = default) { var s = GetStream (); - var statusBytes = await ReadExactBytesIntoNewArrayAsync (s, 4, cancellationToken).ConfigureAwait (false); - // Status is always 4 ASCII chars - if (statusBytes [0] == (byte) 'O' && statusBytes [1] == (byte) 'K' && - statusBytes [2] == (byte) 'A' && statusBytes [3] == (byte) 'Y') + await ReadExactBytesIntoBufferAsync (s, headerBuffer, 4, cancellationToken).ConfigureAwait (false); + if (headerBuffer [0] == (byte) 'O' && headerBuffer [1] == (byte) 'K' && + headerBuffer [2] == (byte) 'A' && headerBuffer [3] == (byte) 'Y') return AdbResponseStatus.Okay; - if (statusBytes [0] == (byte) 'F' && statusBytes [1] == (byte) 'A' && - statusBytes [2] == (byte) 'I' && statusBytes [3] == (byte) 'L') + if (headerBuffer [0] == (byte) 'F' && headerBuffer [1] == (byte) 'A' && + headerBuffer [2] == (byte) 'I' && headerBuffer [3] == (byte) 'L') return AdbResponseStatus.Fail; - var status = Encoding.ASCII.GetString (statusBytes, 0, 4); + var status = Encoding.ASCII.GetString (headerBuffer, 0, 4); throw new InvalidOperationException ($"Unexpected ADB status: '{status}'"); } @@ -116,17 +129,46 @@ public async Task EnsureOkayAsync (CancellationToken cancellationToken = default public async Task ReadLengthPrefixedStringAsync (CancellationToken cancellationToken = default) { var s = GetStream (); - return await ReadLengthPrefixedStringCoreAsync (s, cancellationToken).ConfigureAwait (false); + // Read 4-byte length prefix into reusable buffer + if (!await TryReadExactBytesIntoBufferAsync (s, headerBuffer, 4, cancellationToken).ConfigureAwait (false)) + return null; + + var length = ParseHexLength (headerBuffer); + + if (length == 0) + return string.Empty; + + // Rent from pool, decode to string, return immediately + var buffer = ArrayPool.Shared.Rent (length); + try { + await ReadExactBytesIntoBufferAsync (s, buffer, length, cancellationToken).ConfigureAwait (false); + return Encoding.ASCII.GetString (buffer, 0, length); + } + finally { + ArrayPool.Shared.Return (buffer); + } } /// /// Reads a length-prefixed byte payload from the daemon. /// Returns null if the connection is closed cleanly before the length prefix. + /// The returned byte[] is caller-owned (not pooled). /// public async Task ReadLengthPrefixedBytesAsync (CancellationToken cancellationToken = default) { var s = GetStream (); - return await ReadLengthPrefixedBytesCoreAsync (s, cancellationToken).ConfigureAwait (false); + // Read 4-byte length prefix into reusable buffer + if (!await TryReadExactBytesIntoBufferAsync (s, headerBuffer, 4, cancellationToken).ConfigureAwait (false)) + return null; + + var length = ParseHexLength (headerBuffer); + + if (length == 0) + return Array.Empty (); + + var result = new byte [length]; + await ReadExactBytesIntoBufferAsync (s, result, length, cancellationToken).ConfigureAwait (false); + return result; } /// @@ -170,52 +212,58 @@ void ThrowIfDisposed () throw new ObjectDisposedException (nameof (AdbClient)); } - // --- Shared core implementations (used by both instance and static methods) --- + // --- Shared core implementations (used by static method for tests) --- /// /// Reads a length-prefixed ASCII string from a raw stream. - /// Shared core for both instance and static access patterns. + /// Used by tests that cannot construct an AdbClient instance. + /// Allocates fresh buffers (no pooling) since it has no instance state. /// internal static async Task ReadLengthPrefixedStringFromStreamAsync (Stream stream, CancellationToken cancellationToken) { - return await ReadLengthPrefixedStringCoreAsync (stream, cancellationToken).ConfigureAwait (false); - } - - static async Task ReadLengthPrefixedStringCoreAsync (Stream stream, CancellationToken cancellationToken) - { - var bytes = await ReadLengthPrefixedBytesCoreAsync (stream, cancellationToken).ConfigureAwait (false); - if (bytes == null) - return null; - return Encoding.ASCII.GetString (bytes, 0, bytes.Length); - } - - static async Task ReadLengthPrefixedBytesCoreAsync (Stream stream, CancellationToken cancellationToken) - { - var lengthBytes = await ReadExactBytesOrNullAsync (stream, 4, cancellationToken).ConfigureAwait (false); - if (lengthBytes == null) + var lengthBytes = new byte [4]; + if (!await TryReadExactBytesIntoBufferAsync (stream, lengthBytes, 4, cancellationToken).ConfigureAwait (false)) return null; - var lengthHex = Encoding.ASCII.GetString (lengthBytes, 0, 4); - if (!int.TryParse (lengthHex, System.Globalization.NumberStyles.HexNumber, null, out var length)) - throw new FormatException ($"Invalid ADB length prefix: '{lengthHex}'"); + var length = ParseHexLength (lengthBytes); if (length == 0) - return Array.Empty (); + return string.Empty; - return await ReadExactBytesIntoNewArrayAsync (stream, length, cancellationToken).ConfigureAwait (false); + var payload = new byte [length]; + await ReadExactBytesIntoBufferAsync (stream, payload, length, cancellationToken).ConfigureAwait (false); + return Encoding.ASCII.GetString (payload, 0, length); } - static async Task ReadExactBytesIntoNewArrayAsync (Stream stream, int count, CancellationToken cancellationToken) + // --- Low-level I/O helpers --- + + /// + /// Reads exactly bytes into the provided buffer. + /// Throws IOException if the stream ends prematurely. + /// + static async Task ReadExactBytesIntoBufferAsync (Stream stream, byte[] buffer, int count, CancellationToken cancellationToken) { - var result = await ReadExactBytesOrNullAsync (stream, count, cancellationToken).ConfigureAwait (false); - if (result == null) - throw new IOException ($"Unexpected end of stream (expected {count} bytes)."); - return result; + var totalRead = 0; + while (totalRead < count) { + cancellationToken.ThrowIfCancellationRequested (); +#if NET5_0_OR_GREATER + var read = await stream.ReadAsync (buffer.AsMemory (totalRead, count - totalRead), cancellationToken).ConfigureAwait (false); +#else + var read = await stream.ReadAsync (buffer, totalRead, count - totalRead, cancellationToken).ConfigureAwait (false); +#endif + if (read == 0) + throw new IOException ($"Unexpected end of stream (read {totalRead} of {count} bytes)."); + totalRead += read; + } } - static async Task ReadExactBytesOrNullAsync (Stream stream, int count, CancellationToken cancellationToken) + /// + /// Tries to read exactly bytes into the buffer. + /// Returns false if the stream ends cleanly before the first byte. + /// Throws IOException on partial reads. + /// + static async Task TryReadExactBytesIntoBufferAsync (Stream stream, byte[] buffer, int count, CancellationToken cancellationToken) { - var buffer = new byte [count]; var totalRead = 0; while (totalRead < count) { cancellationToken.ThrowIfCancellationRequested (); @@ -226,11 +274,48 @@ static async Task ReadExactBytesIntoNewArrayAsync (Stream stream, int co #endif if (read == 0) { if (totalRead == 0) - return null; + return false; throw new IOException ($"Unexpected end of stream (read {totalRead} of {count} bytes)."); } totalRead += read; } - return buffer; + return true; + } + + // --- Hex encoding/decoding helpers (avoid string allocations) --- + + static readonly byte[] HexChars = Encoding.ASCII.GetBytes ("0123456789abcdef"); + + /// + /// Writes a 4-digit lowercase hex representation of into the first 4 bytes of . + /// + static void WriteHexLength (byte[] buffer, int value) + { + buffer [0] = HexChars [(value >> 12) & 0xF]; + buffer [1] = HexChars [(value >> 8) & 0xF]; + buffer [2] = HexChars [(value >> 4) & 0xF]; + buffer [3] = HexChars [value & 0xF]; + } + + /// + /// Parses a 4-byte ASCII hex length prefix without allocating a string. + /// + static int ParseHexLength (byte[] buffer) + { + var value = 0; + for (var i = 0; i < 4; i++) { + var b = buffer [i]; + int nibble; + if (b >= (byte) '0' && b <= (byte) '9') + nibble = b - '0'; + else if (b >= (byte) 'a' && b <= (byte) 'f') + nibble = b - 'a' + 10; + else if (b >= (byte) 'A' && b <= (byte) 'F') + nibble = b - 'A' + 10; + else + throw new FormatException ($"Invalid ADB length prefix: '{Encoding.ASCII.GetString (buffer, 0, 4)}'"); + value = (value << 4) | nibble; + } + return value; } }