From cea3d002927c1df167d8c87037b8eb3f142e0bba Mon Sep 17 00:00:00 2001 From: amauryleve Date: Wed, 1 Jul 2026 16:49:05 +0200 Subject: [PATCH 1/3] Add server-initiated session cancellation to the dotnet test IPC protocol (#8691) Implements Option B from the RFC: a reverse "server control" pipe. The SDK advertises a control pipe name in its handshake reply; the test host connects back and parks a long-poll WaitForServerControlRequest that the SDK completes with a ServerControlMessage (CancelSession). On cancel the host stops gracefully via IGracefulStopTestExecutionCapability so trx/artifacts survive, falling back to hard cancellation when no graceful capability exists. A dropped control pipe is treated as "host gone => cancel". - Protocol 1.4.0; handshake property ServerControlPipeName=12; serializer ids WaitForServerControlRequest=13, ServerControlMessage=14. - NamedPipeClient gains exitProcessOnConnectionLoss so the control channel does not kill the host on disconnect. - Contract + round-trip unit tests and end-to-end acceptance tests (graceful cancel still reports; no-advertise is a no-op). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Hosts/CommonTestHost.cs | 27 +++ .../Hosts/TestHostOchestratorHost.cs | 11 + .../IPC/NamedPipeClient.cs | 34 ++++ .../IPC/Serializers/RegisterSerializers.cs | 4 + .../DotnetTest/DotnetTestConnection.cs | 162 ++++++++++++++- .../ServerMode/DotnetTest/IPC/Constants.cs | 26 ++- .../IPC/Models/ServerControlMessage.cs | 11 + .../IPC/Models/WaitForServerControlRequest.cs | 13 ++ .../DotnetTest/IPC/ObjectFieldIds.cs | 14 ++ .../ServerControlMessageSerializer.cs | 64 ++++++ .../WaitForServerControlRequestSerializer.cs | 18 ++ .../ServerMode/IPushOnlyProtocol.cs | 9 + .../DotnetTestPipeBaselineTests.cs | 7 +- .../DotnetTestPipe/DotnetTestPipeProtocol.cs | 33 +++ .../DotnetTestPipeServerCancellationTests.cs | 190 ++++++++++++++++++ .../DotnetTestPipe/FakeDotnetTestSdk.cs | 77 ++++++- .../DotnetTestPipe/FakeDotnetTestSdkResult.cs | 12 +- .../DotnetTestProtocolContractTests.cs | 10 +- .../IPC/ProtocolTests.cs | 41 +++- 19 files changed, 751 insertions(+), 12 deletions(-) create mode 100644 src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/ServerControlMessage.cs create mode 100644 src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/WaitForServerControlRequest.cs create mode 100644 src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/ServerControlMessageSerializer.cs create mode 100644 src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/WaitForServerControlRequestSerializer.cs create mode 100644 test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeServerCancellationTests.cs diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs index 543d6a8e9f..887fe2a83f 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using Microsoft.Testing.Platform.Capabilities.TestFramework; using Microsoft.Testing.Platform.Extensions; using Microsoft.Testing.Platform.Extensions.TestFramework; using Microsoft.Testing.Platform.Extensions.TestHost; @@ -60,6 +61,13 @@ public async Task RunAsync() bool isValidProtocol = await PushOnlyProtocol.IsCompatibleProtocolAsync(hostType).ConfigureAwait(false); + if (isValidProtocol && PushOnlyProtocol.IsServerControlChannelSupported) + { + // Start listening for server-initiated signals (e.g. session cancellation) before running tests + // so a signal that arrives mid-run is observed. React by stopping gracefully where possible. + await PushOnlyProtocol.StartServerControlChannelAsync(RequestGracefulSessionStopAsync).ConfigureAwait(false); + } + exitCode = isValidProtocol ? await RunTestAppAsync(platformOTelService, testApplicationCancellationToken, alreadyDisposed).ConfigureAwait(false) : (int)ExitCode.IncompatibleProtocolVersion; @@ -127,6 +135,25 @@ private string GetHostType() return hostType; } + // Reaction to a server-initiated session cancellation coming over the reverse control pipe. Prefer a graceful + // stop so the framework stops scheduling new tests but still emits trx/logs/artifacts for whatever completed + // (mirroring the local '--maximum-failed-tests' behavior). Fall back to hard cancellation when the running + // framework has no graceful-stop capability (e.g. the test host controller), which is the only lever left. + private async Task RequestGracefulSessionStopAsync(CancellationToken cancellationToken) + { + IGracefulStopTestExecutionCapability? capability = + ServiceProvider.GetService()?.GetCapability(); + + if (capability is not null) + { + await capability.StopTestExecutionAsync(cancellationToken).ConfigureAwait(false); + } + else + { + ServiceProvider.GetTestApplicationCancellationTokenSource().Cancel(); + } + } + private async Task RunTestAppAsync(IPlatformOpenTelemetryService? platformOTelService, CancellationToken testApplicationCancellationToken, List alreadyDisposed) { if (RunTestApplicationLifeCycleCallbacks) diff --git a/src/Platform/Microsoft.Testing.Platform/Hosts/TestHostOchestratorHost.cs b/src/Platform/Microsoft.Testing.Platform/Hosts/TestHostOchestratorHost.cs index a8e63451ba..0449952c83 100644 --- a/src/Platform/Microsoft.Testing.Platform/Hosts/TestHostOchestratorHost.cs +++ b/src/Platform/Microsoft.Testing.Platform/Hosts/TestHostOchestratorHost.cs @@ -46,6 +46,17 @@ public async Task RunAsync() { return (int)ExitCode.IncompatibleProtocolVersion; } + + if (pushOnlyProtocol.IsServerControlChannelSupported) + { + // The orchestrator has no graceful-stop capability of its own; a server-initiated cancel maps to + // cancelling the application token, which propagates to the orchestrated test host processes. + await pushOnlyProtocol.StartServerControlChannelAsync(_ => + { + applicationCancellationToken.Cancel(); + return Task.CompletedTask; + }).ConfigureAwait(false); + } } int exitCode; diff --git a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs index bfcdedd92b..e8ab14d194 100644 --- a/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs +++ b/src/Platform/Microsoft.Testing.Platform/IPC/NamedPipeClient.cs @@ -23,6 +23,7 @@ internal sealed class NamedPipeClient : NamedPipeBase, IClient private readonly NamedPipeClientStream _namedPipeClientStream; private readonly SemaphoreSlim _lock = new(1, 1); private readonly IEnvironment _environment; + private readonly bool _exitProcessOnConnectionLoss; private bool _disposed; @@ -32,6 +33,25 @@ public NamedPipeClient(string name) } public NamedPipeClient(string name, IEnvironment environment) + : this(name, environment, exitProcessOnConnectionLoss: true) + { + } + + /// + /// Initializes a new instance of the class connecting to the named pipe + /// . + /// + /// The OS-level named pipe name to connect to. + /// The environment abstraction used for process-exit on connection loss. + /// + /// When (the default) a lost connection - the server disconnecting while we write a + /// request or before it sends a response - terminates the process via . + /// This is the right behavior for the primary data pipe: if we cannot talk to the host there is no way to + /// recover. Set it to for auxiliary channels (e.g. the reverse server-control pipe) + /// where a dropped connection should surface as an exception the caller can handle cooperatively rather than + /// killing the whole test host. + /// + public NamedPipeClient(string name, IEnvironment environment, bool exitProcessOnConnectionLoss) { if (name is null) { @@ -41,6 +61,7 @@ public NamedPipeClient(string name, IEnvironment environment) _namedPipeClientStream = new(".", name, PipeDirection.InOut, AsyncCurrentUserPipeOptions); PipeName = name; _environment = environment; + _exitProcessOnConnectionLoss = exitProcessOnConnectionLoss; } public string PipeName { get; } @@ -69,6 +90,12 @@ public async Task RequestReplyAsync(TRequest req // The server disconnected while we were writing the request. Mirror the read-EOF handling // below: if we cannot deliver the request there's no way to recover, so exit abnormally // instead of surfacing a raw IPC error to the caller. + if (!_exitProcessOnConnectionLoss) + { + // Auxiliary channel: let the caller observe the disconnect and react cooperatively. + throw; + } + _environment.Exit((int)ExitCode.GenericFailure); throw; } @@ -83,6 +110,13 @@ public async Task RequestReplyAsync(TRequest req // This is especially important for 'dotnet test', where the user can simply kill the dotnet.exe process themselves. // In that case, we want the MTP process to also die. // Exit code 1 indicates abnormal termination due to IPC connection loss. + if (!_exitProcessOnConnectionLoss) + { + // Auxiliary channel (e.g. the reverse server-control pipe): a dropped connection means the + // peer went away. Surface it as an exception so the caller can react (e.g. treat "host gone" + // as a cooperative cancel) instead of killing the whole test host. + throw new IOException($"Pipe '{PipeName}' was closed by the server before a response was received."); + } // Surface a diagnostic on stderr so the user has a chance to understand why this process is exiting. // We deliberately use Console.Error (and not stdout) to avoid corrupting any machine-readable output diff --git a/src/Platform/Microsoft.Testing.Platform/IPC/Serializers/RegisterSerializers.cs b/src/Platform/Microsoft.Testing.Platform/IPC/Serializers/RegisterSerializers.cs index bf01c6d36a..f4b58b70cd 100644 --- a/src/Platform/Microsoft.Testing.Platform/IPC/Serializers/RegisterSerializers.cs +++ b/src/Platform/Microsoft.Testing.Platform/IPC/Serializers/RegisterSerializers.cs @@ -22,6 +22,8 @@ namespace Microsoft.Testing.Platform.IPC.Serializers; * TestInProgressMessagesSerializer: 10 * AzureDevOpsLogMessageSerializer: 11 * DisplayMessageSerializer: 12 + * WaitForServerControlRequestSerializer: 13 + * ServerControlMessageSerializer: 14 */ [Embedded] @@ -41,5 +43,7 @@ public static void RegisterAllSerializers(this NamedPipeBase namedPipeBase) namedPipeBase.RegisterSerializer(new TestInProgressMessagesSerializer(), typeof(TestInProgressMessages)); namedPipeBase.RegisterSerializer(new AzureDevOpsLogMessageSerializer(), typeof(AzureDevOpsLogMessage)); namedPipeBase.RegisterSerializer(new DisplayMessageSerializer(), typeof(DisplayMessage)); + namedPipeBase.RegisterSerializer(new WaitForServerControlRequestSerializer(), typeof(WaitForServerControlRequest)); + namedPipeBase.RegisterSerializer(new ServerControlMessageSerializer(), typeof(ServerControlMessage)); } } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs index dcbc7595d2..c59f51d9fa 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs @@ -23,6 +23,12 @@ internal sealed class DotnetTestConnection : IPushOnlyProtocol, IDisposable private NamedPipeClient? _dotnetTestPipeClient; + private NamedPipeClient? _serverControlPipeClient; + private Task? _serverControlListenerTask; + private CancellationTokenSource? _serverControlListenerCts; + private string? _serverControlPipeName; + private int _cancelRequested; + public static string InstanceId { get; } = Guid.NewGuid().ToString("N"); public DotnetTestConnection(CommandLineHandler commandLineHandler, IEnvironment environment, ITestApplicationModuleInfo testApplicationModuleInfo, ITestApplicationCancellationTokenSource cancellationTokenSource) @@ -97,6 +103,12 @@ public async Task HelpInvokedAsync() // so an older SDK (<= 1.2.0) never receives an unknown message id. public bool IsDisplayMessageForwardingSupported { get; private set; } + // True once the SDK advertised a reverse "server control" pipe name in its handshake reply. When set, the + // test host opens a NamedPipeClient to that pipe and parks a long-poll so the SDK can push a + // ServerControlMessage (e.g. CancelSession) at any time. The feature is gated on the presence of the handshake + // property (a capability), not on the negotiated version string, so an older SDK simply never enables it. + public bool IsServerControlChannelSupported { get; private set; } + public async Task IsCompatibleProtocolAsync(string hostType, IReadOnlyDictionary? additionalHandshakeProperties = null) { RoslynDebug.Assert(_dotnetTestPipeClient is not null); @@ -132,6 +144,13 @@ public async Task IsCompatibleProtocolAsync(string hostType, IReadOnlyDict bool.TryParse(isIDEValue, out bool isIDE) && isIDE; + if (response.Properties?.TryGetValue(HandshakeMessagePropertyNames.ServerControlPipeName, out string? serverControlPipeName) == true && + !RoslynString.IsNullOrEmpty(serverControlPipeName)) + { + _serverControlPipeName = serverControlPipeName; + IsServerControlChannelSupported = true; + } + if (response.Properties?.TryGetValue(HandshakeMessagePropertyNames.SupportedProtocolVersions, out string? protocolVersion) is true) { bool isCompatible = IsVersionCompatible(protocolVersion, supportedProtocolVersions); @@ -186,7 +205,146 @@ public async Task SendMessageAsync(IRequest message) } } - public Task OnExitAsync() => Task.CompletedTask; + /// + /// Opens the reverse "server control" pipe the SDK advertised during the handshake and parks a long-poll + /// on it. The SDK completes that request with a + /// whenever it wants to signal the test host (today only + /// ). On cancel - or when the control pipe drops, which means + /// the host went away - is invoked exactly once so the caller + /// can stop the run cooperatively (preferring a graceful stop so trx/artifacts are still produced). + /// + /// + /// The reaction to a server-initiated cancel. It receives the test application cancellation token. + /// + /// + /// This is best-effort: if the control pipe cannot be established the test run continues unaffected (the + /// feature simply stays off). Callers should only invoke this after a successful handshake and when + /// is . + /// + public async Task StartServerControlChannelAsync(Func onCancelSessionRequestedAsync) + { + if (!IsServerControlChannelSupported || RoslynString.IsNullOrEmpty(_serverControlPipeName) || _serverControlPipeClient is not null) + { + return; + } + + _serverControlListenerCts = CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.CancellationToken); + + // exitProcessOnConnectionLoss: false - a dropped control pipe must not kill the test host; the listener + // turns it into a cooperative cancel instead. + var controlClient = new NamedPipeClient(_serverControlPipeName, _environment, exitProcessOnConnectionLoss: false); + controlClient.RegisterAllSerializers(); + + try + { + // Bound the connect so a misbehaving SDK that advertises a pipe it never listens on cannot hang the run. + using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(_serverControlListenerCts.Token); + connectCts.CancelAfter(TimeSpan.FromSeconds(30)); + await controlClient.ConnectAsync(connectCts.Token).ConfigureAwait(false); + } + catch (Exception) + { + // Best-effort: failing to establish the control channel degrades to "no server-initiated cancel" + // rather than breaking the test run. + controlClient.Dispose(); + return; + } + + _serverControlPipeClient = controlClient; + _serverControlListenerTask = Task.Run( + () => ListenForServerControlAsync(controlClient, onCancelSessionRequestedAsync, _serverControlListenerCts.Token)); + } + + private async Task ListenForServerControlAsync(NamedPipeClient controlClient, Func onCancelSessionRequestedAsync, CancellationToken cancellationToken) + { + try + { + while (!cancellationToken.IsCancellationRequested) + { + ServerControlMessage message = await controlClient.RequestReplyAsync( + WaitForServerControlRequest.CachedInstance, cancellationToken).ConfigureAwait(false); + + if (message.Kind == ServerControlKinds.CancelSession) + { + await RequestCancelOnceAsync(onCancelSessionRequestedAsync).ConfigureAwait(false); + return; + } + + // Unknown control kind (forward-compat): ignore it and keep parking for the next signal. + } + } + catch (OperationCanceledException) + { + // We are shutting down (dispose or app cancellation) - nothing to do. + } + catch (Exception) when (!cancellationToken.IsCancellationRequested) + { + // The control pipe dropped while the session was still live => the host went away. Treat it as a + // cooperative cancel so we still try to wind down and report whatever completed. + await RequestCancelOnceAsync(onCancelSessionRequestedAsync).ConfigureAwait(false); + } + catch (Exception) + { + // Cancellation raced with a pipe error; ignore. + } + } + + private async Task RequestCancelOnceAsync(Func onCancelSessionRequestedAsync) + { + if (Interlocked.Exchange(ref _cancelRequested, 1) != 0) + { + return; + } + + await onCancelSessionRequestedAsync(_cancellationTokenSource.CancellationToken).ConfigureAwait(false); + } + + public async Task OnExitAsync() + { +#if NET + if (_serverControlListenerCts is { } cts) + { + await cts.CancelAsync().ConfigureAwait(false); + } +#else +#pragma warning disable VSTHRD103 // CancellationTokenSource.CancelAsync is not available on this target framework. + _serverControlListenerCts?.Cancel(); +#pragma warning restore VSTHRD103 +#endif + + if (_serverControlListenerTask is { } listenerTask) + { + try + { + await listenerTask.ConfigureAwait(false); + } + catch (Exception) + { + // The listener swallows its own expected exceptions; anything reaching here happened during + // shutdown and must not fail the exit path. + } + } + } + + public void Dispose() + { + _serverControlListenerCts?.Cancel(); - public void Dispose() => _dotnetTestPipeClient?.Dispose(); + if (_serverControlListenerTask is { } listenerTask) + { + try + { + // Bounded wait: cancelling the linked token unblocks the parked read quickly. Never hang exit. + listenerTask.Wait(TimeSpan.FromSeconds(5)); + } + catch (Exception) + { + // Best-effort shutdown. + } + } + + _serverControlPipeClient?.Dispose(); + _serverControlListenerCts?.Dispose(); + _dotnetTestPipeClient?.Dispose(); + } } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs index 700e820a8f..6e8dff604f 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs @@ -61,6 +61,23 @@ internal static class HandshakeMessagePropertyNames // dotnet test in the SDK) can understand why an orchestrator is participating // in the run. The value is the orchestrator extension Uid. internal const byte OrchestratorFeature = 11; + + // Carries the OS-level name of the reverse "server control" pipe. Only ever sent by the SDK + // (dotnet test) in its handshake reply. Its presence is the capability signal for + // server-initiated session cancellation: when the test host sees a non-empty value it opens a + // NamedPipeClient to that pipe and parks a long-poll WaitForServerControlRequest so the SDK can + // push a ServerControlMessage (e.g. CancelSession) at any time - even while the test host is + // otherwise silent. An older SDK never sends this property, so the feature stays disabled. + internal const byte ServerControlPipeName = 12; +} + +[Embedded] +internal static class ServerControlKinds +{ + // The kind of a ServerControlMessage the SDK pushes to the test host over the reverse control pipe. + // Values must stay stable (they flow over IPC to dotnet test). Reserve additional values for future + // signals (drain, pause, ...). + internal const byte CancelSession = 1; } [Embedded] @@ -128,5 +145,12 @@ internal static class ProtocolConstants // output (the SDK suppresses its TerminalTestReporter to avoid colliding with the host output it expected // before this change). Users must update to an SDK that negotiates 1.1.0 to see live output via the SDK's // TerminalTestReporter. - internal const string SupportedVersions = "1.0.0;1.1.0;1.2.0;1.3.0"; + // 1.4.0 adds the reverse "server control" channel used for server-initiated session cancellation. When the + // SDK advertises a ServerControlPipeName in its handshake reply, the test host opens a NamedPipeClient to that + // pipe and parks a long-poll WaitForServerControlRequest; the SDK completes it with a ServerControlMessage + // (e.g. CancelSession) whenever it wants the test host to stop cooperatively (global --maximum-failed-tests, + // --timeout, ...). The feature is gated on the presence of the handshake property (a capability), not on this + // version string, so an older SDK that never advertises the pipe leaves the feature disabled. The version is + // still bumped so the negotiated-version state advances in lockstep. + internal const string SupportedVersions = "1.0.0;1.1.0;1.2.0;1.3.0;1.4.0"; } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/ServerControlMessage.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/ServerControlMessage.cs new file mode 100644 index 0000000000..5aaa004379 --- /dev/null +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/ServerControlMessage.cs @@ -0,0 +1,11 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.Testing.Platform.IPC.Models; + +// The SDK's reply to a parked WaitForServerControlRequest on the reverse "server control" pipe. Kind is one of +// ServerControlKinds (e.g. CancelSession); on CancelSession the test host stops scheduling new tests and stops the +// current run cooperatively (preferring a graceful stop so trx/artifacts are still produced). Introduced with +// protocol version 1.4.0; the SDK only creates the control pipe when it advertises ServerControlPipeName, so an +// older SDK never sends this message. +internal sealed record ServerControlMessage(byte Kind) : IResponse; diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/WaitForServerControlRequest.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/WaitForServerControlRequest.cs new file mode 100644 index 0000000000..611765770b --- /dev/null +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Models/WaitForServerControlRequest.cs @@ -0,0 +1,13 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.Testing.Platform.IPC.Models; + +// Sent once by the test host on the reverse "server control" pipe (see HandshakeMessagePropertyNames. +// ServerControlPipeName). The test host parks this long-poll request and the SDK completes it with a +// ServerControlMessage whenever it wants to push a control signal (e.g. CancelSession). Introduced with +// protocol version 1.4.0; the request carries no payload. +internal sealed class WaitForServerControlRequest : IRequest +{ + public static readonly WaitForServerControlRequest CachedInstance = new(); +} diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/ObjectFieldIds.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/ObjectFieldIds.cs index 4acf89b1d6..631a17e14d 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/ObjectFieldIds.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/ObjectFieldIds.cs @@ -197,3 +197,17 @@ internal static class DisplayMessageFieldsId public const ushort Level = 3; public const ushort Text = 4; } + +[Embedded] +internal static class WaitForServerControlRequestFieldsId +{ + public const int MessagesSerializerId = 13; +} + +[Embedded] +internal static class ServerControlMessageFieldsId +{ + public const int MessagesSerializerId = 14; + + public const ushort Kind = 1; +} diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/ServerControlMessageSerializer.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/ServerControlMessageSerializer.cs new file mode 100644 index 0000000000..aed9c01b9b --- /dev/null +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/ServerControlMessageSerializer.cs @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Testing.Platform.IPC.Models; + +namespace Microsoft.Testing.Platform.IPC.Serializers; + +/* + |---FieldCount---| 2 bytes + + |---Kind Id---| (2 bytes) + |---Kind Size---| (4 bytes) + |---Kind Value---| (1 byte) +*/ + +internal sealed class ServerControlMessageSerializer : NamedPipeSerializer, INamedPipeSerializer +{ + public override int Id => ServerControlMessageFieldsId.MessagesSerializerId; + + protected override ServerControlMessage DeserializeCore(Stream stream) + { + byte kind = 0; + + ushort fieldCount = ReadUShort(stream); + + for (int i = 0; i < fieldCount; i++) + { + ushort fieldId = ReadUShort(stream); + int fieldSize = ReadInt(stream); + + switch (fieldId) + { + case ServerControlMessageFieldsId.Kind: + kind = ReadByte(stream); + + // Kind is a single byte today, but honor the declared field size so that a future + // protocol revision that widens it (or a frame that reports a different size) does not + // leave extra bytes unread and misalign the remaining fields. + if (fieldSize > 1) + { + SetPosition(stream, stream.Position + (fieldSize - 1)); + } + + break; + + default: + // If we don't recognize the field id, skip the payload corresponding to that field. + SetPosition(stream, stream.Position + fieldSize); + break; + } + } + + return new(kind); + } + + protected override void SerializeCore(ServerControlMessage objectToSerialize, Stream stream) + { + RoslynDebug.Assert(stream.CanSeek, "We expect a seekable stream."); + + // Kind is always written (it is a non-nullable byte). + WriteUShort(stream, 1); + WriteField(stream, ServerControlMessageFieldsId.Kind, objectToSerialize.Kind); + } +} diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/WaitForServerControlRequestSerializer.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/WaitForServerControlRequestSerializer.cs new file mode 100644 index 0000000000..ba60b549b8 --- /dev/null +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Serializers/WaitForServerControlRequestSerializer.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Testing.Platform.IPC.Models; + +namespace Microsoft.Testing.Platform.IPC.Serializers; + +internal sealed class WaitForServerControlRequestSerializer : NamedPipeSerializer, INamedPipeSerializer +{ + public override int Id => WaitForServerControlRequestFieldsId.MessagesSerializerId; + + protected override WaitForServerControlRequest DeserializeCore(Stream _) + => WaitForServerControlRequest.CachedInstance; + + protected override void SerializeCore(WaitForServerControlRequest _, Stream __) + { + } +} diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/IPushOnlyProtocol.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/IPushOnlyProtocol.cs index e5c6173245..334e94b153 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/IPushOnlyProtocol.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/IPushOnlyProtocol.cs @@ -7,12 +7,21 @@ internal interface IPushOnlyProtocol : IDisposable { bool IsServerMode { get; } + // True once the SDK advertised a reverse "server control" pipe during the handshake, meaning it can push + // server-initiated signals (e.g. session cancellation) to the test host. Only meaningful after a successful + // IsCompatibleProtocolAsync call. + bool IsServerControlChannelSupported { get; } + Task AfterCommonServiceSetupAsync(); Task HelpInvokedAsync(); Task IsCompatibleProtocolAsync(string testHostType, IReadOnlyDictionary? additionalHandshakeProperties = null); + // Opens the reverse "server control" channel (when supported) and parks a long-poll on it. The provided + // reaction runs exactly once when the SDK signals a session cancellation (or when the control pipe drops). + Task StartServerControlChannelAsync(Func onCancelSessionRequestedAsync); + Task GetDataConsumerAsync(); Task OnExitAsync(); diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeBaselineTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeBaselineTests.cs index e685e28fa2..3b1bf00946 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeBaselineTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeBaselineTests.cs @@ -25,11 +25,12 @@ public class DotnetTestPipeBaselineTests : AcceptanceTestBase properties) return stream.ToArray(); } + /// + /// Encodes the body of a frame. Mirrors + /// ServerControlMessageSerializer: a field-tagged object with a single Kind field (id 1) + /// carrying one byte (). + /// + public static byte[] EncodeServerControlMessageBody(byte kind) + { + const ushort kindFieldId = 1; + + using MemoryStream stream = new(); + WriteUShort(stream, 1); // field count + WriteUShort(stream, kindFieldId); + WriteInt(stream, sizeof(byte)); // field size + stream.WriteByte(kind); + + return stream.ToArray(); + } + /// /// Decodes the body of a frame. /// Format: ushort fieldCount; (ushort fieldId, int fieldSize, payload)*fieldCount @@ -545,6 +571,13 @@ private static void WriteUShort(Stream stream, ushort value) stream.Write(bytes); } + private static void WriteInt(Stream stream, int value) + { + Span bytes = stackalloc byte[sizeof(int)]; + BitConverter.TryWriteBytes(bytes, value); + stream.Write(bytes); + } + private static string ReadLengthPrefixedString(Stream stream) { int length = ReadInt(stream); diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeServerCancellationTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeServerCancellationTests.cs new file mode 100644 index 0000000000..e8491e0c74 --- /dev/null +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/DotnetTestPipeServerCancellationTests.cs @@ -0,0 +1,190 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.Testing.Platform.Acceptance.IntegrationTests.DotnetTestPipe; + +/// +/// End-to-end tests for server-initiated session cancellation over the --server dotnettestcli pipe +/// protocol (issue https://github.com/microsoft/testfx/issues/8691). +/// +/// The fake SDK advertises a reverse "server control" pipe in its handshake reply. The test app connects back to +/// it and parks a long-poll; the SDK completes that long-poll with a CancelSession message while the app is +/// blocked inside a running test. The app is expected to react by stopping gracefully - it still emits its +/// test results and TestSessionEnd and exits cleanly, rather than being killed. +/// +/// +[TestClass] +public sealed class DotnetTestPipeServerCancellationTests : AcceptanceTestBase +{ + private const string AssetName = "DotnetTestPipeServerCancellationTest"; + + public TestContext TestContext { get; set; } = null!; + + [TestMethod] + public async Task DotnetTestPipe_ServerCancelSession_StopsGracefullyAndStillReports() + { + var testHost = TestInfrastructure.TestHost.LocateFrom( + AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetCurrent); + + // The fake SDK advertises the reverse control pipe and, once the app connects and parks, pushes a + // CancelSession. The advertised protocol version must include 1.4.0 so the negotiation succeeds; the + // feature itself is gated on the ServerControlPipeName property, not the version string. + FakeDotnetTestSdkResult result = await FakeDotnetTestSdk.RunAsync( + testHost, + environmentVariables: new Dictionary { ["SERVERCANCEL_WAIT"] = "1" }, + supportedProtocolVersions: "1.0.0;1.1.0;1.2.0;1.3.0;1.4.0", + advertiseServerControlPipe: true, + cancellationToken: TestContext.CancellationToken); + + Assert.IsTrue(result.ServerControlPipeConnected, "The test app should have connected back to the reverse server-control pipe."); + Assert.IsTrue(result.ServerCancelSent, "The fake SDK should have pushed a CancelSession over the control pipe."); + + // Reporting must survive the cancel: the app publishes a passed result only AFTER it observes the cancel, + // so seeing a TestResult frame proves the graceful path kept the reporting pipeline alive. + Assert.IsNotEmpty( + result.MessagesWithSerializerId(DotnetTestPipeProtocol.SerializerIds.TestResultMessages).ToList(), + "Expected a TestResult message after the server-initiated cancel (reporting should survive a graceful stop)."); + + // TestSessionEnd must still be emitted - the app wound down cleanly instead of being killed. + byte[] sessionEventTypes = + [ + .. result.MessagesWithSerializerId(DotnetTestPipeProtocol.SerializerIds.TestSessionEvent) + .Select(m => DotnetTestPipeProtocol.DecodeTestSessionEventBody(m.Body).SessionType) + .Where(t => t.HasValue) + .Select(t => t!.Value) + ]; + + Assert.Contains( + DotnetTestPipeProtocol.SessionEventTypes.TestSessionEnd, + sessionEventTypes, + $"Expected a TestSessionEnd event after cancel; got [{string.Join(", ", sessionEventTypes)}]."); + + Assert.AreEqual( + 0, + result.TestHostResult.ExitCode, + $"A graceful server-initiated cancel that still reports a passing test should exit cleanly.{Environment.NewLine}" + + $"stdout:{Environment.NewLine}{result.TestHostResult.StandardOutput}{Environment.NewLine}" + + $"stderr:{Environment.NewLine}{result.TestHostResult.StandardError}"); + } + + [TestMethod] + public async Task DotnetTestPipe_NoServerControlPipeAdvertised_AppNeverConnectsBack() + { + var testHost = TestInfrastructure.TestHost.LocateFrom( + AssetFixture.TargetAssetPath, AssetName, TargetFrameworks.NetCurrent); + + // An SDK that does not advertise the control pipe (the old-SDK / opted-out case) must not cause the app + // to open any control channel; the feature stays off and the app finishes on its own fallback timeout. + FakeDotnetTestSdkResult result = await FakeDotnetTestSdk.RunAsync( + testHost, + supportedProtocolVersions: "1.0.0;1.1.0;1.2.0;1.3.0;1.4.0", + advertiseServerControlPipe: false, + cancellationToken: TestContext.CancellationToken); + + Assert.IsFalse(result.ServerControlPipeConnected, "No control pipe was advertised, so the app must not connect back."); + Assert.IsFalse(result.ServerCancelSent, "No control pipe was advertised, so no cancel could be sent."); + } + + public sealed class TestAssetFixture() : TestAssetFixtureBase() + { + private const string AssetCode = """ +#file DotnetTestPipeServerCancellationTest.csproj + + + $TargetFrameworks$ + enable + enable + Exe + true + preview + + $(NoWarn);TPEXP + + + + + + + +#file Program.cs +using Microsoft.Testing.Platform.Builder; +using Microsoft.Testing.Platform.Capabilities.TestFramework; +using Microsoft.Testing.Platform.Extensions.Messages; +using Microsoft.Testing.Platform.Extensions.TestFramework; + +public class Program +{ + public static async Task Main(string[] args) + { + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); + var framework = new DummyTestFramework(); + builder.RegisterTestFramework( + _ => new TestFrameworkCapabilities(framework.GracefulStopCapability), + (_, __) => framework); + using ITestApplication app = await builder.BuildAsync(); + return await app.RunAsync(); + } +} + +// A graceful-stop capability whose StopTestExecutionAsync unblocks the running test. The platform resolves this +// capability when it receives a server-initiated CancelSession over the reverse control pipe. +public sealed class DummyGracefulStopCapability : IGracefulStopTestExecutionCapability +{ + private readonly TaskCompletionSource _requested = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public Task Requested => _requested.Task; + + public Task StopTestExecutionAsync(CancellationToken cancellationToken) + { + _requested.TrySetResult(); + return Task.CompletedTask; + } +} + +public sealed class DummyTestFramework : ITestFramework, IDataProducer +{ + public DummyGracefulStopCapability GracefulStopCapability { get; } = new(); + + public string Uid => nameof(DummyTestFramework); + public string Version => "2.0.0"; + public string DisplayName => nameof(DummyTestFramework); + public string Description => nameof(DummyTestFramework); + public Type[] DataTypesProduced => new[] { typeof(TestNodeUpdateMessage) }; + public Task IsEnabledAsync() => Task.FromResult(true); + + public Task CreateTestSessionAsync(CreateTestSessionContext context) + => Task.FromResult(new CreateTestSessionResult() { IsSuccess = true }); + + public Task CloseTestSessionAsync(CloseTestSessionContext context) + => Task.FromResult(new CloseTestSessionResult() { IsSuccess = true }); + + public async Task ExecuteRequestAsync(ExecuteRequestContext context) + { + // Only the cancellation scenario asks us to park (via an env var) so the negative test finishes fast. + if (Environment.GetEnvironmentVariable("SERVERCANCEL_WAIT") == "1") + { + // Park until the SDK asks us to stop (via the reverse control pipe -> graceful stop) or the whole + // application is cancelled. A generous fallback timeout keeps the process from hanging forever if no + // signal ever arrives (the test assertions then fail loudly rather than the run deadlocking). + Task requested = GracefulStopCapability.Requested; + Task timeout = Task.Delay(TimeSpan.FromSeconds(60)); + await Task.WhenAny(requested, timeout); + } + + // Reporting must survive the cancel: publish a passed result AFTER we observed the stop request so the + // harness can prove the graceful path kept the reporting pipeline alive. + await context.MessageBus.PublishAsync(this, new TestNodeUpdateMessage(context.Request.Session.SessionUid, + new TestNode() { Uid = "1", DisplayName = "CancelledButReported", Properties = new(PassedTestNodeStateProperty.CachedInstance) })); + context.Complete(); + } +} +"""; + + public string TargetAssetPath => GetAssetPath(AssetName); + + public override (string ID, string Name, string Code) GetAssetsToGenerate() => (AssetName, AssetName, + AssetCode + .PatchTargetFrameworks(TargetFrameworks.NetCurrent) + .PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion)); + } +} diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs index 47f524f8c0..03615dce58 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs @@ -33,6 +33,7 @@ public static async Task RunAsync( Dictionary? environmentVariables = null, string supportedProtocolVersions = DefaultSupportedProtocolVersions, bool isIde = false, + bool advertiseServerControlPipe = false, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(testHost); @@ -49,10 +50,22 @@ public static async Task RunAsync( // path (matching what NamedPipeServer.GetPipeName produces with Path.Combine("/tmp", name)). using NamedPipeServerStream stream = new(osPipeName, PipeDirection.InOut, maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, options); + // Optional reverse "server control" pipe: when advertised, the test app connects back and parks a + // WaitForServerControlRequest that we complete with a CancelSession, exercising server-initiated + // session cancellation (issue #8691). + string? controlOsPipeName = null; + NamedPipeServerStream? controlStream = null; + if (advertiseServerControlPipe) + { + controlOsPipeName = DotnetTestPipeProtocol.GetPipeName(Guid.NewGuid().ToString("N")); + controlStream = new(controlOsPipeName, PipeDirection.InOut, maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, options); + } + List received = []; Dictionary? receivedHandshake = null; Dictionary? sentHandshakeReply = null; string? negotiatedVersion = null; + var controlObservations = new ServerControlObservations(); string pipeArgs = $"--server dotnettestcli --dotnet-test-pipe {osPipeName}"; string finalArgs = extraArguments is null ? pipeArgs : $"{pipeArgs} {extraArguments}"; @@ -61,6 +74,11 @@ public static async Task RunAsync( // Wait for the test app to connect. await stream.WaitForConnectionAsync(cancellationToken).ConfigureAwait(false); + // Accept the control connection and push the cancel signal concurrently with the data read loop. + Task? controlTask = controlStream is null + ? null + : Task.Run(() => DriveServerControlAsync(controlStream, controlObservations, cancellationToken), cancellationToken); + // Read frames until the peer disconnects. while (true) { @@ -77,7 +95,7 @@ public static async Task RunAsync( receivedHandshake = DotnetTestPipeProtocol.DecodeHandshakeBody(frame.Body); string selected = SelectHighestMutuallySupportedVersion(receivedHandshake, supportedProtocolVersions); negotiatedVersion = selected; - sentHandshakeReply = BuildSdkHandshakeReply(selected, isIde); + sentHandshakeReply = BuildSdkHandshakeReply(selected, isIde, controlOsPipeName); byte[] replyBody = DotnetTestPipeProtocol.EncodeHandshakeBody(sentHandshakeReply); await DotnetTestPipeProtocol.WriteFrameAsync(stream, DotnetTestPipeProtocol.SerializerIds.HandshakeMessage, replyBody, cancellationToken).ConfigureAwait(false); } @@ -90,12 +108,60 @@ public static async Task RunAsync( TestHostResult hostResult = await hostRun.ConfigureAwait(false); + if (controlTask is not null) + { + try + { + await controlTask.WaitAsync(TimeSpan.FromSeconds(30), cancellationToken).ConfigureAwait(false); + } + catch (Exception) + { + // Best-effort: the control interaction is observed via controlObservations; don't let its + // teardown fail the harness. + } + } + + if (controlStream is not null) + { + await controlStream.DisposeAsync().ConfigureAwait(false); + } + return new FakeDotnetTestSdkResult( hostResult, received, receivedHandshake, sentHandshakeReply, - negotiatedVersion); + negotiatedVersion, + controlObservations.Connected, + controlObservations.CancelSent); + } + + /// + /// Accepts the reverse control-pipe connection, reads the single parked + /// , and completes it with a + /// message. + /// + private static async Task DriveServerControlAsync(NamedPipeServerStream controlStream, ServerControlObservations observations, CancellationToken cancellationToken) + { + await controlStream.WaitForConnectionAsync(cancellationToken).ConfigureAwait(false); + observations.Connected = true; + + RawMessage? request = await DotnetTestPipeProtocol.ReadFrameAsync(controlStream, cancellationToken).ConfigureAwait(false); + if (request is null || request.SerializerId != DotnetTestPipeProtocol.SerializerIds.WaitForServerControlRequest) + { + return; + } + + byte[] body = DotnetTestPipeProtocol.EncodeServerControlMessageBody(DotnetTestPipeProtocol.ServerControlKinds.CancelSession); + await DotnetTestPipeProtocol.WriteFrameAsync(controlStream, DotnetTestPipeProtocol.SerializerIds.ServerControlMessage, body, cancellationToken).ConfigureAwait(false); + observations.CancelSent = true; + } + + private sealed class ServerControlObservations + { + public bool Connected { get; set; } + + public bool CancelSent { get; set; } } /// @@ -141,7 +207,7 @@ internal static string SelectHighestMutuallySupportedVersion(Dictionary BuildSdkHandshakeReply(string selectedVersion, bool isIde) + private static Dictionary BuildSdkHandshakeReply(string selectedVersion, bool isIde, string? serverControlPipeName = null) { Dictionary properties = new(capacity: 6) { @@ -157,6 +223,11 @@ private static Dictionary BuildSdkHandshakeReply(string selectedVe properties.Add(DotnetTestPipeProtocol.HandshakeProperties.IsIDE, bool.TrueString); } + if (!string.IsNullOrEmpty(serverControlPipeName)) + { + properties.Add(DotnetTestPipeProtocol.HandshakeProperties.ServerControlPipeName, serverControlPipeName); + } + return properties; } } diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdkResult.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdkResult.cs index f3df515849..a7a4184331 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdkResult.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdkResult.cs @@ -16,13 +16,17 @@ public FakeDotnetTestSdkResult( IReadOnlyList receivedMessages, Dictionary? receivedHandshake, Dictionary? sentHandshakeReply, - string? negotiatedProtocolVersion) + string? negotiatedProtocolVersion, + bool serverControlPipeConnected = false, + bool serverCancelSent = false) { TestHostResult = testHostResult; ReceivedMessages = receivedMessages; ReceivedHandshake = receivedHandshake; SentHandshakeReply = sentHandshakeReply; NegotiatedProtocolVersion = negotiatedProtocolVersion; + ServerControlPipeConnected = serverControlPipeConnected; + ServerCancelSent = serverCancelSent; } /// The process-level result: exit code, captured stdout, captured stderr. @@ -41,6 +45,12 @@ public FakeDotnetTestSdkResult( /// The version the fake SDK selected from the test app's advertised list, or null/empty if none. public string? NegotiatedProtocolVersion { get; } + /// True when the test app connected back to the reverse server-control pipe the SDK advertised. + public bool ServerControlPipeConnected { get; } + + /// True when the fake SDK pushed a CancelSession control message to the test app. + public bool ServerCancelSent { get; } + /// Returns all frames whose serializer ID matches . public IEnumerable MessagesWithSerializerId(int serializerId) => ReceivedMessages.Where(m => m.SerializerId == serializerId); diff --git a/test/UnitTests/Microsoft.Testing.Platform.DotnetTestProtocolContract.UnitTests/DotnetTestProtocolContractTests.cs b/test/UnitTests/Microsoft.Testing.Platform.DotnetTestProtocolContract.UnitTests/DotnetTestProtocolContractTests.cs index 5a7021990d..77519bf874 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.DotnetTestProtocolContract.UnitTests/DotnetTestProtocolContractTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.DotnetTestProtocolContract.UnitTests/DotnetTestProtocolContractTests.cs @@ -39,6 +39,8 @@ public void SerializerIds_AreStable() [TestInProgressMessagesFieldsId.MessagesSerializerId] = nameof(TestInProgressMessagesFieldsId), [AzureDevOpsLogMessageFieldsId.MessagesSerializerId] = nameof(AzureDevOpsLogMessageFieldsId), [DisplayMessageFieldsId.MessagesSerializerId] = nameof(DisplayMessageFieldsId), + [WaitForServerControlRequestFieldsId.MessagesSerializerId] = nameof(WaitForServerControlRequestFieldsId), + [ServerControlMessageFieldsId.MessagesSerializerId] = nameof(ServerControlMessageFieldsId), }; Assert.AreEqual(nameof(VoidResponseFieldsId), serializerIds[0]); @@ -55,6 +57,8 @@ public void SerializerIds_AreStable() Assert.AreEqual(nameof(TestInProgressMessagesFieldsId), serializerIds[10]); Assert.AreEqual(nameof(AzureDevOpsLogMessageFieldsId), serializerIds[11]); Assert.AreEqual(nameof(DisplayMessageFieldsId), serializerIds[12]); + Assert.AreEqual(nameof(WaitForServerControlRequestFieldsId), serializerIds[13]); + Assert.AreEqual(nameof(ServerControlMessageFieldsId), serializerIds[14]); } [TestMethod] @@ -73,6 +77,8 @@ public void HandshakeMessagePropertyNames_AreStable() [HandshakeMessagePropertyNames.InstanceId] = nameof(HandshakeMessagePropertyNames.InstanceId), [HandshakeMessagePropertyNames.IsIDE] = nameof(HandshakeMessagePropertyNames.IsIDE), [HandshakeMessagePropertyNames.ExecutionMode] = nameof(HandshakeMessagePropertyNames.ExecutionMode), + [HandshakeMessagePropertyNames.OrchestratorFeature] = nameof(HandshakeMessagePropertyNames.OrchestratorFeature), + [HandshakeMessagePropertyNames.ServerControlPipeName] = nameof(HandshakeMessagePropertyNames.ServerControlPipeName), }; Assert.AreEqual(nameof(HandshakeMessagePropertyNames.PID), properties[0]); @@ -86,6 +92,8 @@ public void HandshakeMessagePropertyNames_AreStable() Assert.AreEqual(nameof(HandshakeMessagePropertyNames.InstanceId), properties[8]); Assert.AreEqual(nameof(HandshakeMessagePropertyNames.IsIDE), properties[9]); Assert.AreEqual(nameof(HandshakeMessagePropertyNames.ExecutionMode), properties[10]); + Assert.AreEqual(nameof(HandshakeMessagePropertyNames.OrchestratorFeature), properties[11]); + Assert.AreEqual(nameof(HandshakeMessagePropertyNames.ServerControlPipeName), properties[12]); } [TestMethod] @@ -142,6 +150,6 @@ public void ProtocolVersion_IsStable() // Indirect through a collection so the MSTest analyzer does not flag the comparison of a compile-time // constant as "always true" (MSTEST0032). string[] versions = [ProtocolConstants.SupportedVersions]; - Assert.AreEqual("1.0.0;1.1.0;1.2.0;1.3.0", versions[0]); + Assert.AreEqual("1.0.0;1.1.0;1.2.0;1.3.0;1.4.0", versions[0]); } } diff --git a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs index a23d667e7d..602d69b16b 100644 --- a/test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs +++ b/test/UnitTests/Microsoft.Testing.Platform.UnitTests/IPC/ProtocolTests.cs @@ -85,6 +85,39 @@ public void DisplayMessageSerializeDeserialize_WithNullOptionalFields() Assert.IsNull(actual.Text); } + [TestMethod] + public void ServerControlMessageSerializeDeserialize() + { + object serializer = new ServerControlMessageSerializer(); + var message = new ServerControlMessage(ServerControlKinds.CancelSession); + + ServerControlMessage actual = RoundTrip(serializer, message); + + Assert.AreEqual(ServerControlKinds.CancelSession, actual.Kind); + } + + [TestMethod] + public void ServerControlMessageSerializeDeserialize_UnknownKindIsPreserved() + { + object serializer = new ServerControlMessageSerializer(); + // A forward-compat kind the current host does not recognize must still round-trip its byte value. + var message = new ServerControlMessage(42); + + ServerControlMessage actual = RoundTrip(serializer, message); + + Assert.AreEqual((byte)42, actual.Kind); + } + + [TestMethod] + public void WaitForServerControlRequestSerializeDeserialize() + { + object serializer = new WaitForServerControlRequestSerializer(); + + WaitForServerControlRequest actual = RoundTrip(serializer, WaitForServerControlRequest.CachedInstance); + + Assert.IsNotNull(actual); + } + [TestMethod] public void DiscoveredTestMessagesSerializeDeserialize() { @@ -230,6 +263,7 @@ public void HandshakeMessagePropertyNames_ValuesAreStable() { HandshakeMessagePropertyNames.IsIDE, nameof(HandshakeMessagePropertyNames.IsIDE) }, { HandshakeMessagePropertyNames.ExecutionMode, nameof(HandshakeMessagePropertyNames.ExecutionMode) }, { HandshakeMessagePropertyNames.OrchestratorFeature, nameof(HandshakeMessagePropertyNames.OrchestratorFeature) }, + { HandshakeMessagePropertyNames.ServerControlPipeName, nameof(HandshakeMessagePropertyNames.ServerControlPipeName) }, }; Assert.AreEqual(nameof(HandshakeMessagePropertyNames.PID), properties[0]); @@ -244,6 +278,7 @@ public void HandshakeMessagePropertyNames_ValuesAreStable() Assert.AreEqual(nameof(HandshakeMessagePropertyNames.IsIDE), properties[9]); Assert.AreEqual(nameof(HandshakeMessagePropertyNames.ExecutionMode), properties[10]); Assert.AreEqual(nameof(HandshakeMessagePropertyNames.OrchestratorFeature), properties[11]); + Assert.AreEqual(nameof(HandshakeMessagePropertyNames.ServerControlPipeName), properties[12]); } // The HandshakeMessageExecutionModes string values flow over IPC to @@ -416,6 +451,8 @@ public void SerializerIds_AreStable() [TestInProgressMessagesFieldsId.MessagesSerializerId] = nameof(TestInProgressMessagesFieldsId), [AzureDevOpsLogMessageFieldsId.MessagesSerializerId] = nameof(AzureDevOpsLogMessageFieldsId), [DisplayMessageFieldsId.MessagesSerializerId] = nameof(DisplayMessageFieldsId), + [WaitForServerControlRequestFieldsId.MessagesSerializerId] = nameof(WaitForServerControlRequestFieldsId), + [ServerControlMessageFieldsId.MessagesSerializerId] = nameof(ServerControlMessageFieldsId), }; Assert.AreEqual(nameof(VoidResponseFieldsId), serializerIds[0]); @@ -432,6 +469,8 @@ public void SerializerIds_AreStable() Assert.AreEqual(nameof(TestInProgressMessagesFieldsId), serializerIds[10]); Assert.AreEqual(nameof(AzureDevOpsLogMessageFieldsId), serializerIds[11]); Assert.AreEqual(nameof(DisplayMessageFieldsId), serializerIds[12]); + Assert.AreEqual(nameof(WaitForServerControlRequestFieldsId), serializerIds[13]); + Assert.AreEqual(nameof(ServerControlMessageFieldsId), serializerIds[14]); } // The SessionEventTypes byte values flow over IPC to dotnet test in the dotnet/sdk repository. @@ -487,6 +526,6 @@ public void ProtocolVersion_IsStable() // Indirect through a collection so the MSTest analyzer does not flag the comparison of a compile-time // constant as "always true" (MSTEST0032). string[] versions = [ProtocolConstants.SupportedVersions]; - Assert.AreEqual("1.0.0;1.1.0;1.2.0;1.3.0", versions[0]); + Assert.AreEqual("1.0.0;1.1.0;1.2.0;1.3.0;1.4.0", versions[0]); } } From 72be67eb3df9ead91b32b34a8363c2d2d674f909 Mon Sep 17 00:00:00 2001 From: amauryleve Date: Wed, 1 Jul 2026 17:08:56 +0200 Subject: [PATCH 2/3] Address expert review: background the control connect and force-abort the parked read on exit - StartServerControlChannelAsync no longer blocks test start on the control-pipe connect; connect + long-poll now run entirely on the background listener task (fixes up-to-30s startup stall and de-risks multi-process contention). - OnExitAsync now cancels then disposes the control pipe client before a bounded wait, so the parked long-poll read is force-aborted even on .NET Framework where cancelling an in-flight named-pipe read is unreliable (fixes potential exit hang). - Documented the SDK-side contract for ServerControlPipeName (one control connection per connecting process; SDK must keep the pipe open for the whole data session, since an early drop is treated as cancel). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../DotnetTest/DotnetTestConnection.cs | 65 ++++++++++++------- .../ServerMode/DotnetTest/IPC/Constants.cs | 9 +++ 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs index c59f51d9fa..000cca026a 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs @@ -217,15 +217,19 @@ public async Task SendMessageAsync(IRequest message) /// The reaction to a server-initiated cancel. It receives the test application cancellation token. /// /// - /// This is best-effort: if the control pipe cannot be established the test run continues unaffected (the - /// feature simply stays off). Callers should only invoke this after a successful handshake and when - /// is . + /// Both the connect and the long-poll run entirely on a background task, so test start is never blocked on + /// this auxiliary channel. This is best-effort: if the control pipe cannot be established the test run + /// continues unaffected (the feature simply stays off). Callers should only invoke this after a successful + /// handshake and when is . It is invoked + /// once per connection on the awaited run path (before the test run starts), so the control-channel fields + /// below are written here and only read later during exit/dispose - the single-threaded lifecycle makes the + /// plain fields safe without extra synchronization. /// - public async Task StartServerControlChannelAsync(Func onCancelSessionRequestedAsync) + public Task StartServerControlChannelAsync(Func onCancelSessionRequestedAsync) { if (!IsServerControlChannelSupported || RoslynString.IsNullOrEmpty(_serverControlPipeName) || _serverControlPipeClient is not null) { - return; + return Task.CompletedTask; } _serverControlListenerCts = CancellationTokenSource.CreateLinkedTokenSource(_cancellationTokenSource.CancellationToken); @@ -234,25 +238,33 @@ public async Task StartServerControlChannelAsync(Func o // turns it into a cooperative cancel instead. var controlClient = new NamedPipeClient(_serverControlPipeName, _environment, exitProcessOnConnectionLoss: false); controlClient.RegisterAllSerializers(); + _serverControlPipeClient = controlClient; + + // Connect AND listen on a background task: we deliberately do not await the connect on the run path so a + // slow/absent control server can never delay (or fail) test execution start. + _serverControlListenerTask = Task.Run( + () => ConnectAndListenForServerControlAsync(controlClient, onCancelSessionRequestedAsync, _serverControlListenerCts.Token)); + return Task.CompletedTask; + } + private async Task ConnectAndListenForServerControlAsync(NamedPipeClient controlClient, Func onCancelSessionRequestedAsync, CancellationToken cancellationToken) + { try { - // Bound the connect so a misbehaving SDK that advertises a pipe it never listens on cannot hang the run. - using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(_serverControlListenerCts.Token); + // Bound the connect so a misbehaving SDK that advertises a pipe it never listens on cannot leave this + // task parked forever holding resources. + using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); connectCts.CancelAfter(TimeSpan.FromSeconds(30)); await controlClient.ConnectAsync(connectCts.Token).ConfigureAwait(false); } catch (Exception) { // Best-effort: failing to establish the control channel degrades to "no server-initiated cancel" - // rather than breaking the test run. - controlClient.Dispose(); + // rather than affecting the test run. return; } - _serverControlPipeClient = controlClient; - _serverControlListenerTask = Task.Run( - () => ListenForServerControlAsync(controlClient, onCancelSessionRequestedAsync, _serverControlListenerCts.Token)); + await ListenForServerControlAsync(controlClient, onCancelSessionRequestedAsync, cancellationToken).ConfigureAwait(false); } private async Task ListenForServerControlAsync(NamedPipeClient controlClient, Func onCancelSessionRequestedAsync, CancellationToken cancellationToken) @@ -280,12 +292,14 @@ private async Task ListenForServerControlAsync(NamedPipeClient controlClient, Fu catch (Exception) when (!cancellationToken.IsCancellationRequested) { // The control pipe dropped while the session was still live => the host went away. Treat it as a - // cooperative cancel so we still try to wind down and report whatever completed. + // cooperative cancel so we still try to wind down and report whatever completed. NOTE: this makes it a + // protocol requirement that the SDK keep the control pipe open until the data session ends - an early + // close for any reason is interpreted here as a cancel. await RequestCancelOnceAsync(onCancelSessionRequestedAsync).ConfigureAwait(false); } catch (Exception) { - // Cancellation raced with a pipe error; ignore. + // Cancellation raced with a pipe error (e.g. the stream was disposed during teardown); ignore. } } @@ -312,17 +326,16 @@ public async Task OnExitAsync() #pragma warning restore VSTHRD103 #endif + // Cancelling the token is enough to abort an in-flight named-pipe read on modern .NET, but on .NET + // Framework cancellation of an already-parked overlapped read is not reliable - disposing the stream is + // what forces it to unblock. We cancelled first (above) so the listener treats the resulting failure as + // shutdown rather than "host gone => cancel". + _serverControlPipeClient?.Dispose(); + if (_serverControlListenerTask is { } listenerTask) { - try - { - await listenerTask.ConfigureAwait(false); - } - catch (Exception) - { - // The listener swallows its own expected exceptions; anything reaching here happened during - // shutdown and must not fail the exit path. - } + // Bounded wait so a stuck listener can never hang the exit path on any target framework. + await Task.WhenAny(listenerTask, Task.Delay(TimeSpan.FromSeconds(5))).ConfigureAwait(false); } } @@ -330,11 +343,14 @@ public void Dispose() { _serverControlListenerCts?.Cancel(); + // Disposing the pipe client forces a parked read to abort even where token cancellation alone would not. + _serverControlPipeClient?.Dispose(); + if (_serverControlListenerTask is { } listenerTask) { try { - // Bounded wait: cancelling the linked token unblocks the parked read quickly. Never hang exit. + // Bounded wait: the cancel + dispose above unblock the parked read quickly. Never hang exit. listenerTask.Wait(TimeSpan.FromSeconds(5)); } catch (Exception) @@ -343,7 +359,6 @@ public void Dispose() } } - _serverControlPipeClient?.Dispose(); _serverControlListenerCts?.Dispose(); _dotnetTestPipeClient?.Dispose(); } diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs index 6e8dff604f..145ed7b998 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/IPC/Constants.cs @@ -68,6 +68,15 @@ internal static class HandshakeMessagePropertyNames // NamedPipeClient to that pipe and parks a long-poll WaitForServerControlRequest so the SDK can // push a ServerControlMessage (e.g. CancelSession) at any time - even while the test host is // otherwise silent. An older SDK never sends this property, so the feature stays disabled. + // + // SDK-side contract (duplicated by hand in dotnet/sdk - keep in sync): + // * Every process that performs the handshake and receives this property (test host, test host + // controller, orchestrator) opens its own client to the advertised name, so the SDK must be able to + // accept one control connection per connecting process (e.g. one server instance per process, or a + // distinct pipe name advertised per handshake reply). + // * The SDK MUST keep the control pipe open for the whole data session. The test host treats an early + // pipe drop as "host gone => cancel", so closing the control pipe before the data session ends would be + // interpreted as a cancellation. internal const byte ServerControlPipeName = 12; } From 2100f68f464a964e656661dd6d25689387811929 Mon Sep 17 00:00:00 2001 From: amauryleve Date: Thu, 2 Jul 2026 20:02:38 +0200 Subject: [PATCH 3/3] Address PR review: reliable control-stream disposal and idiomatic null-safe check - FakeDotnetTestSdk: declare the reverse control-pipe server stream with 'await using' so its OS handle is released on every exit path (incl. exceptions), removing the manual DisposeAsync block. - DotnetTestConnection: use 'is true' instead of '== true' for the nullable-bool handshake property check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../DotnetTest/DotnetTestConnection.cs | 2 +- .../DotnetTestPipe/FakeDotnetTestSdk.cs | 19 ++++++------------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs index 000cca026a..438c135093 100644 --- a/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs +++ b/src/Platform/Microsoft.Testing.Platform/ServerMode/DotnetTest/DotnetTestConnection.cs @@ -144,7 +144,7 @@ public async Task IsCompatibleProtocolAsync(string hostType, IReadOnlyDict bool.TryParse(isIDEValue, out bool isIDE) && isIDE; - if (response.Properties?.TryGetValue(HandshakeMessagePropertyNames.ServerControlPipeName, out string? serverControlPipeName) == true && + if (response.Properties?.TryGetValue(HandshakeMessagePropertyNames.ServerControlPipeName, out string? serverControlPipeName) is true && !RoslynString.IsNullOrEmpty(serverControlPipeName)) { _serverControlPipeName = serverControlPipeName; diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs index 03615dce58..7e68d68820 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/DotnetTestPipe/FakeDotnetTestSdk.cs @@ -52,14 +52,12 @@ public static async Task RunAsync( // Optional reverse "server control" pipe: when advertised, the test app connects back and parks a // WaitForServerControlRequest that we complete with a CancelSession, exercising server-initiated - // session cancellation (issue #8691). - string? controlOsPipeName = null; - NamedPipeServerStream? controlStream = null; - if (advertiseServerControlPipe) - { - controlOsPipeName = DotnetTestPipeProtocol.GetPipeName(Guid.NewGuid().ToString("N")); - controlStream = new(controlOsPipeName, PipeDirection.InOut, maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, options); - } + // session cancellation (issue #8691). Declared with 'await using' so the OS handle is released reliably + // on every exit path (including exceptions), and remains a no-op when the feature is not advertised. + string? controlOsPipeName = advertiseServerControlPipe ? DotnetTestPipeProtocol.GetPipeName(Guid.NewGuid().ToString("N")) : null; + await using NamedPipeServerStream? controlStream = controlOsPipeName is null + ? null + : new(controlOsPipeName, PipeDirection.InOut, maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, options); List received = []; Dictionary? receivedHandshake = null; @@ -121,11 +119,6 @@ public static async Task RunAsync( } } - if (controlStream is not null) - { - await controlStream.DisposeAsync().ConfigureAwait(false); - } - return new FakeDotnetTestSdkResult( hostResult, received,