Skip to content

Commit 94514ae

Browse files
committed
Improve shutdown cleanup and CTS disposal
ScreencastService: Unsubscribe page close handler and cancel the frame pump immediately to avoid a circular dependency between the pump and page disposal; leave full cleanup to StopAsync/DisposeAsync. Browser: Mark connection as closed earlier, unregister signal handlers, and dispose the transport to terminate the WebSocket receive loop before waiting on the browser process. Use a dedicated 5s CancellationTokenSource when waiting for process exit and kill the process if it doesn't exit in time. NetworkManager: Ensure the linked CancellationTokenSource is disposed when waiter tasks complete (success, timeout, or cancellation) to avoid CTS leaks in WaitForRequest/WaitForResponse.
1 parent 0bfaa68 commit 94514ae

3 files changed

Lines changed: 23 additions & 4 deletions

File tree

src/Motus.Runner/Services/ScreencastService.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,14 @@ await _page.StartScreencastAsync(
4646

4747
private void OnPageClose(object? sender, EventArgs e)
4848
{
49-
_ = StopAsync();
49+
// Unsubscribe immediately and cancel the frame pump.
50+
// Full cleanup (awaiting _pumpTask, StopScreencastAsync) happens in
51+
// StopAsync/DisposeAsync to avoid a circular dependency where the pump
52+
// blocks on a CDP channel that is only completed after page disposal.
53+
if (sender is InternalPage page)
54+
page.Close -= OnPageClose;
55+
56+
_pumpCts?.Cancel();
5057
}
5158

5259
public async Task HighlightElementAsync(string? objectId, CancellationToken ct = default)

src/Motus/Browser/Browser.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,10 @@ public async Task CloseAsync()
7575
if (!_isConnected)
7676
return;
7777

78+
_isConnected = false;
79+
80+
UnregisterSignalHandlers();
81+
7882
// Close all contexts first
7983
List<BrowserContext> contextsToClose;
8084
lock (_contexts)
@@ -100,19 +104,21 @@ await _registry.BrowserSession.SendAsync(
100104
// Expected: browser closes the WebSocket on shutdown
101105
}
102106

107+
// Dispose the transport to terminate the WebSocket receive loop
108+
await _transport.DisposeAsync().ConfigureAwait(false);
109+
103110
if (_process is not null && !_process.HasExited)
104111
{
112+
using var exitCts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
105113
try
106114
{
107-
await _process.WaitForExitAsync(new CancellationTokenSource(TimeSpan.FromSeconds(5)).Token).ConfigureAwait(false);
115+
await _process.WaitForExitAsync(exitCts.Token).ConfigureAwait(false);
108116
}
109117
catch (OperationCanceledException)
110118
{
111119
_process.Kill(entireProcessTree: true);
112120
}
113121
}
114-
115-
_isConnected = false;
116122
}
117123

118124
public async ValueTask DisposeAsync()

src/Motus/Network/NetworkManager.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,9 @@ internal Task<IRequest> WaitForRequestAsync(string urlPattern, TimeSpan timeout)
113113
cts.Token.Register(() => tcs.TrySetException(
114114
new TimeoutException($"WaitForRequest timed out after {timeout.TotalMilliseconds}ms.")));
115115

116+
// Dispose the linked CTS once the waiter completes (success, timeout, or cancellation)
117+
tcs.Task.ContinueWith(_ => cts.Dispose(), TaskContinuationOptions.ExecuteSynchronously);
118+
116119
return tcs.Task;
117120
}
118121

@@ -127,6 +130,9 @@ internal Task<IResponse> WaitForResponseAsync(string urlPattern, TimeSpan timeou
127130
cts.Token.Register(() => tcs.TrySetException(
128131
new TimeoutException($"WaitForResponse timed out after {timeout.TotalMilliseconds}ms.")));
129132

133+
// Dispose the linked CTS once the waiter completes (success, timeout, or cancellation)
134+
tcs.Task.ContinueWith(_ => cts.Dispose(), TaskContinuationOptions.ExecuteSynchronously);
135+
130136
return tcs.Task;
131137
}
132138

0 commit comments

Comments
 (0)