Skip to content

Commit 440a867

Browse files
committed
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.
1 parent 84b4edd commit 440a867

3 files changed

Lines changed: 87 additions & 15 deletions

File tree

TUnit.Playwright/BrowserService.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,14 @@ private static async Task<IBrowser> CreateBrowser(
5151
}
5252

5353
public Task ResetAsync() => Task.CompletedTask;
54-
public Task DisposeAsync() => Browser.CloseAsync();
54+
55+
public async Task DisposeAsync()
56+
{
57+
var browser = Browser;
58+
59+
if (browser != null)
60+
{
61+
await browser.CloseAsync().ConfigureAwait(false);
62+
}
63+
}
5564
}

TUnit.Playwright/BrowserTest.cs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,18 @@ public BrowserTest(BrowserTypeLaunchOptions options)
1717
public IBrowser Browser { get; internal set; } = null!;
1818

1919
private readonly List<IBrowserContext> _contexts = [];
20+
private readonly object _contextsLock = new();
2021
private readonly BrowserTypeLaunchOptions _options;
2122

2223
public async Task<IBrowserContext> NewContext(BrowserNewContextOptions options)
2324
{
2425
var context = await Browser.NewContextAsync(options).ConfigureAwait(false);
25-
_contexts.Add(context);
26+
27+
lock (_contextsLock)
28+
{
29+
_contexts.Add(context);
30+
}
31+
2632
return context;
2733
}
2834

@@ -33,22 +39,42 @@ public async Task BrowserSetup()
3339
{
3440
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.");
3541
}
36-
42+
3743
var service = await BrowserService.Register(this, BrowserType, _options).ConfigureAwait(false);
3844
Browser = service.Browser;
3945
}
4046

4147
[After(HookType.Test, "", 0)]
4248
public async Task BrowserTearDown(TestContext testContext)
4349
{
44-
if (TestOk(testContext))
50+
List<IBrowserContext> contextsSnapshot;
51+
52+
lock (_contextsLock)
53+
{
54+
contextsSnapshot = [.. _contexts];
55+
_contexts.Clear();
56+
}
57+
58+
List<Exception>? exceptions = null;
59+
60+
foreach (var context in contextsSnapshot)
4561
{
46-
foreach (var context in _contexts)
62+
try
4763
{
4864
await context.CloseAsync().ConfigureAwait(false);
4965
}
66+
catch (Exception ex)
67+
{
68+
exceptions ??= [];
69+
exceptions.Add(ex);
70+
}
5071
}
51-
_contexts.Clear();
72+
5273
Browser = null!;
74+
75+
if (exceptions is { Count: > 0 })
76+
{
77+
throw new AggregateException("One or more browser contexts failed to close.", exceptions);
78+
}
5379
}
5480
}

TUnit.Playwright/WorkerAwareTest.cs

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ private class Worker
2222

2323
public async Task<T> RegisterService<T>(string name, Func<Task<T>> factory) where T : class, IWorkerService
2424
{
25-
if (!_currentWorker.Services.ContainsKey(name))
25+
if (!_currentWorker.Services.TryGetValue(name, out var existing))
2626
{
27-
_currentWorker.Services[name] = await factory().ConfigureAwait(false);
27+
var service = await factory().ConfigureAwait(false);
28+
_currentWorker.Services[name] = service;
29+
return service;
2830
}
2931

30-
return (_currentWorker.Services[name] as T)!;
32+
return (existing as T)!;
3133
}
3234

3335
[Before(HookType.Test, "", 0)]
@@ -44,23 +46,58 @@ public void WorkerSetup()
4446
[After(HookType.Test, "", 0)]
4547
public async Task WorkerTeardown(TestContext testContext)
4648
{
49+
var worker = _currentWorker;
50+
51+
if (worker == null)
52+
{
53+
return;
54+
}
55+
4756
if (TestOk(testContext))
4857
{
49-
foreach (var kv in _currentWorker.Services)
58+
try
5059
{
51-
await kv.Value.ResetAsync().ConfigureAwait(false);
52-
}
60+
foreach (var kv in worker.Services)
61+
{
62+
await kv.Value.ResetAsync().ConfigureAwait(false);
63+
}
5364

54-
AllWorkers.Push(_currentWorker);
65+
AllWorkers.Push(worker);
66+
}
67+
catch
68+
{
69+
await DisposeAllServicesAsync(worker).ConfigureAwait(false);
70+
throw;
71+
}
5572
}
5673
else
5774
{
58-
foreach (var kv in _currentWorker.Services)
75+
await DisposeAllServicesAsync(worker).ConfigureAwait(false);
76+
}
77+
}
78+
79+
private static async Task DisposeAllServicesAsync(Worker worker)
80+
{
81+
List<Exception>? exceptions = null;
82+
83+
foreach (var kv in worker.Services)
84+
{
85+
try
5986
{
6087
await kv.Value.DisposeAsync().ConfigureAwait(false);
6188
}
89+
catch (Exception ex)
90+
{
91+
exceptions ??= [];
92+
exceptions.Add(ex);
93+
}
94+
}
6295

63-
_currentWorker.Services.Clear();
96+
worker.Services.Clear();
97+
98+
if (exceptions is { Count: > 0 })
99+
{
100+
throw new AggregateException("One or more worker services failed to dispose.", exceptions);
64101
}
65102
}
66103

0 commit comments

Comments
 (0)