From bd8dbdeba749862fccdedd342b19930371c7dd8e Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Thu, 19 Feb 2026 00:41:31 +0000 Subject: [PATCH] fix: resolve Playwright resource leaks and improve disposal robustness Browser contexts were not closed on test failure paths in BrowserTest, leading to leaked browser resources. Additionally, if any single context or service disposal threw an exception, remaining resources were skipped entirely. This change ensures all contexts and services are always disposed via try/catch loops that collect exceptions, adds null-safety to BrowserService.DisposeAsync, protects the context list with a lock, and falls back to full disposal when WorkerAwareTest.ResetAsync fails. --- TUnit.Playwright/BrowserService.cs | 11 +++++- TUnit.Playwright/BrowserTest.cs | 36 ++++++++++++++++--- TUnit.Playwright/WorkerAwareTest.cs | 55 ++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 15 deletions(-) diff --git a/TUnit.Playwright/BrowserService.cs b/TUnit.Playwright/BrowserService.cs index 7eb2a35daf..22f9b66860 100644 --- a/TUnit.Playwright/BrowserService.cs +++ b/TUnit.Playwright/BrowserService.cs @@ -51,5 +51,14 @@ private static async Task CreateBrowser( } public Task ResetAsync() => Task.CompletedTask; - public Task DisposeAsync() => Browser.CloseAsync(); + + public async Task DisposeAsync() + { + var browser = Browser; + + if (browser != null) + { + await browser.CloseAsync().ConfigureAwait(false); + } + } } diff --git a/TUnit.Playwright/BrowserTest.cs b/TUnit.Playwright/BrowserTest.cs index 13f00b6e33..1bd00c363b 100644 --- a/TUnit.Playwright/BrowserTest.cs +++ b/TUnit.Playwright/BrowserTest.cs @@ -17,12 +17,18 @@ public BrowserTest(BrowserTypeLaunchOptions options) public IBrowser Browser { get; internal set; } = null!; private readonly List _contexts = []; + private readonly object _contextsLock = new(); private readonly BrowserTypeLaunchOptions _options; public async Task NewContext(BrowserNewContextOptions options) { var context = await Browser.NewContextAsync(options).ConfigureAwait(false); - _contexts.Add(context); + + lock (_contextsLock) + { + _contexts.Add(context); + } + return context; } @@ -33,7 +39,7 @@ public async Task BrowserSetup() { throw new InvalidOperationException($"BrowserType is not initialized. This may indicate that {nameof(PlaywrightTest)}.{nameof(Playwright)} is not initialized or {nameof(PlaywrightTest)}.{nameof(PlaywrightSetup)} did not execute properly."); } - + var service = await BrowserService.Register(this, BrowserType, _options).ConfigureAwait(false); Browser = service.Browser; } @@ -41,14 +47,34 @@ public async Task BrowserSetup() [After(HookType.Test, "", 0)] public async Task BrowserTearDown(TestContext testContext) { - if (TestOk(testContext)) + List contextsSnapshot; + + lock (_contextsLock) + { + contextsSnapshot = [.. _contexts]; + _contexts.Clear(); + } + + List? exceptions = null; + + foreach (var context in contextsSnapshot) { - foreach (var context in _contexts) + try { await context.CloseAsync().ConfigureAwait(false); } + catch (Exception ex) + { + exceptions ??= []; + exceptions.Add(ex); + } } - _contexts.Clear(); + Browser = null!; + + if (exceptions is { Count: > 0 }) + { + throw new AggregateException("One or more browser contexts failed to close.", exceptions); + } } } diff --git a/TUnit.Playwright/WorkerAwareTest.cs b/TUnit.Playwright/WorkerAwareTest.cs index d83475c10c..efb738aff4 100644 --- a/TUnit.Playwright/WorkerAwareTest.cs +++ b/TUnit.Playwright/WorkerAwareTest.cs @@ -22,12 +22,14 @@ private class Worker public async Task RegisterService(string name, Func> factory) where T : class, IWorkerService { - if (!_currentWorker.Services.ContainsKey(name)) + if (!_currentWorker.Services.TryGetValue(name, out var existing)) { - _currentWorker.Services[name] = await factory().ConfigureAwait(false); + var service = await factory().ConfigureAwait(false); + _currentWorker.Services[name] = service; + return service; } - return (_currentWorker.Services[name] as T)!; + return (existing as T)!; } [Before(HookType.Test, "", 0)] @@ -44,23 +46,58 @@ public void WorkerSetup() [After(HookType.Test, "", 0)] public async Task WorkerTeardown(TestContext testContext) { + var worker = _currentWorker; + + if (worker == null) + { + return; + } + if (TestOk(testContext)) { - foreach (var kv in _currentWorker.Services) + try { - await kv.Value.ResetAsync().ConfigureAwait(false); - } + foreach (var kv in worker.Services) + { + await kv.Value.ResetAsync().ConfigureAwait(false); + } - AllWorkers.Push(_currentWorker); + AllWorkers.Push(worker); + } + catch + { + await DisposeAllServicesAsync(worker).ConfigureAwait(false); + throw; + } } else { - foreach (var kv in _currentWorker.Services) + await DisposeAllServicesAsync(worker).ConfigureAwait(false); + } + } + + private static async Task DisposeAllServicesAsync(Worker worker) + { + List? exceptions = null; + + foreach (var kv in worker.Services) + { + try { await kv.Value.DisposeAsync().ConfigureAwait(false); } + catch (Exception ex) + { + exceptions ??= []; + exceptions.Add(ex); + } + } - _currentWorker.Services.Clear(); + worker.Services.Clear(); + + if (exceptions is { Count: > 0 }) + { + throw new AggregateException("One or more worker services failed to dispose.", exceptions); } }