Fix double disposal of ITestHostApplicationLifetime implementing IAsyncCleanableExtension
Fix double call to IAsyncCleanableExtension::CleanupAsync method
Issue: When an MTP extension implements both ITestHostApplicationLifetime and IAsyncCleanableExtension, the CleanupAsync method is called twice - once after AfterRunAsync in RunTestAppAsync and again in DisposeServiceProviderAsync.
Changes Made:
- [x] Analyze the issue and understand the code flow
- [x] Track disposed
ITestHostApplicationLifetimeservices inRunTestAppAsyncusing a field_alreadyDisposedServices - [x] Pass the list of already disposed services to
DisposeServiceProviderAsyncto prevent double disposal - [x] Fix redundant duplicate check for
ITestHostApplicationLifetimeon lines 246-247 - [x] Fix existing bug where
dataConsumerwas not being added toalreadyDisposedlist (was addingserviceinstead) - [x] Add unit tests for DisposeHelper to prevent regression
- [x] Update tests to use MSTest assertions (Assert.AreEqual)
- [x] Run code review (completed with no issues)
- [x] Run security scan (completed with no issues)
Technical Details:
- Added
_alreadyDisposedServicesfield to track services that have been disposed inRunTestAppAsync - Modified
RunTestAppAsyncto add eachITestHostApplicationLifetimeservice to_alreadyDisposedServicesafter disposing it - Modified
RunAsyncto pass_alreadyDisposedServicestoDisposeServiceProviderAsync -
DisposeServiceProviderAsyncalready has logic to skip services in thealreadyDisposedlist, preventing double disposal - Removed duplicate
ITestHostApplicationLifetimecheck in the service disposal condition - Fixed pre-existing bug where
dataConsumerwas not being tracked inalreadyDisposedlist (line 274) - Added comprehensive unit tests in
DisposeHelperTests.csto verify disposal behavior and prevent regression - Updated tests to use MSTest assertions following project conventions
Security Summary: No security vulnerabilities were introduced by these changes. CodeQL analysis found no issues.
Original prompt
This section details on the original issue you should resolve
<issue_title>
IAsyncCleanableExtension::CleanupAsyncmethod is called twice</issue_title> <issue_description>## Describe the bug When MTP extension implements bothITestHostApplicationLifetime/IAsyncCleanableExtension.IAsyncCleanableExtension::CleanupAsyncmethod seems to be called twice.It's confirmed it can reproduced on version
2.0.0-preview.25372.6. And it also reproduced with version1.7.2(UsingITestApplicationLifecycleCallbacks)Steps To Reproduce
1. Create console application 2. Reference Microsoft.Testing.Platform
2.0.0-preview.25372.6package 3. Paste following test code and run console app
using Microsoft.Testing.Platform.Builder; using Microsoft.Testing.Platform.Capabilities.TestFramework; using Microsoft.Testing.Platform.Extensions; using Microsoft.Testing.Platform.Extensions.Messages; using Microsoft.Testing.Platform.Extensions.TestFramework; using Microsoft.Testing.Platform.Extensions.TestHost; using Microsoft.Testing.Platform.Requests; namespace TestConsoleApp; #pragma warning disable TPEXP internal class Program { static async Task<int> Main(string[] args) { var builder = await TestApplication.CreateBuilderAsync(args); var testFramework = new DummyTestAdapter(); builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_, _) => testFramework); var myExtension = new MyExtension(testFramework); builder.TestHost.AddTestHostApplicationLifetime(_ => myExtension); // builder.TestHost.AddDataConsumer(_ => myExtension); var app = await builder.BuildAsync(); return await app.RunAsync(); } } internal class MyExtension(IExtension extension) : ITestHostApplicationLifetime, IAsyncCleanableExtension { public Task<bool> IsEnabledAsync() => extension.IsEnabledAsync(); public string Uid => extension.Uid; public string Version => extension.Version; public string DisplayName => extension.DisplayName; public string Description => extension.Description; public Task BeforeRunAsync(CancellationToken cancellationToken) { Console.WriteLine("BeforeRunAsync"); return Task.CompletedTask; } public Task AfterRunAsync(int exitCode, CancellationToken cancellation) { Console.WriteLine("AfterRunAsync"); return Task.CompletedTask; } public Task CleanupAsync() { Console.WriteLine("CleanupAsync"); return Task.CompletedTask; } } internal class DummyTestAdapter : ITestFramework, IDataProducer { public Task<bool> IsEnabledAsync() => Task.FromResult(true); public string Uid => nameof(DummyTestAdapter); public string Version => string.Empty; public string DisplayName => string.Empty; public string Description => string.Empty; public Type[] DataTypesProduced => [typeof(TestNodeUpdateMessage)]; public Task<CloseTestSessionResult> CloseTestSessionAsync(CloseTestSessionContext context) => Task.FromResult(new CloseTestSessionResult { IsSuccess = true }); public Task<CreateTestSessionResult> CreateTestSessionAsync(CreateTestSessionContext context) => Task.FromResult(new CreateTestSessionResult { IsSuccess = true }); public async Task ExecuteRequestAsync(ExecuteRequestContext context) { switch (context.Request) { case DiscoverTestExecutionRequest discoverRequest: case RunTestExecutionRequest runRequest: await Task.Yield(); //await HandleRequest(context); break; default: break; } context.Complete(); } }Expected behavior
CleanupAsyncis expected to be called exactly once. BecauseCleanupAsyncmethod has no special note that it may be called multiple times likeDispose/DisposeAsync.Actual behavior
CleanupAsynclog outputted twice..Additional context
It seems to be disposed at following lines.
- https://github.com/microsoft/testfx/blob/v4.0.0-preview.25372.6/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs#L123
- https://github.com/microsoft/testfx/blob/v4.0.0-preview.25372.6/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs#L256
Additionally, Following lines seems redundant because it checks same types. https://github.com/microsoft/testfx/blob/v4.0.0-preview.25372.6/src/Platform/Microsoft.Testing.Platform/Hosts/CommonTestHost.cs#L246-L247</issue_description>
Comments on the Issue (you are @copilot in this section)
- Fixes microsoft/testfx#6181
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.
@copilot let's add a test so we don't regress
Added unit tests in DisposeHelperTests.cs to verify that CleanupAsync is called correctly and to prevent regression. The tests verify:
- Extensions implementing
IAsyncCleanableExtensionhaveCleanupAsynccalled exactly once - Extensions implementing both
ITestHostApplicationLifetimeandIAsyncCleanableExtensionare properly handled - The disposal flow works correctly
Commit: 2253eaf
[!WARNING]
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
7tjvsblobprodcus341.vsblob.vsassets.io
- Triggering command:
/home/REDACTED/work/testfx/testfx/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/testfx/testfx/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/testfx/testfx/artifacts/toolset/11.0.0-beta.25524.1.txt(dns block)If you need me to access, download, or install something from one of these locations, you can either:
- Configure Actions setup steps to set up my environment, which run before the firewall is enabled
- Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)
Moving back to draft for manual validation.