From 6db854dfe8c27cc255f7723dd49867b67eeb098a Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 10 Nov 2026 12:49:57 +0000 Subject: [PATCH] fix(threading): run message dispatch and library code on one dispatcher thread The client keeps the JS method bodies, which rely on the event loop: nothing happens between a state check and the event subscription that follows it. Each connection now has a dispatcher thread that restores this. - Driver messages and library code run on the dispatcher thread, one item at a time. - An API call hops onto it in WrapApiCallAsync and returns on the thread pool. - Completion sources are completed after the current message, as a JS microtask. - Event accessors replace the handler list atomically on the caller's thread; the dispatcher thread alone tells the server which events to send. - Lists that getters hand out are copy-on-write. See docs/threading.md for the model and the rules. Fixes: https://github.com/microsoft/playwright-dotnet/issues/3364 Fixes: https://github.com/microsoft/playwright-dotnet/issues/3399 --- CLAUDE.md | 7 + docs/threading.md | 56 +++ .../BrowserContextBasicTests.cs | 14 + src/Playwright.Tests/DispatcherTests.cs | 247 +++++++++++ src/Playwright.Tests/FrameHierarchyTests.cs | 20 + .../PageRunAndWaitForResponseTests.cs | 14 + .../PageWaitForLoadStateTests.cs | 28 ++ src/Playwright.Tests/PageWaitForUrlTests.cs | 75 ++++ .../PlaywrightDisposeTests.cs | 19 + src/Playwright.Tests/ThreadingModelTests.cs | 397 ++++++++++++++++++ src/Playwright/Core/APIRequestContext.cs | 16 +- src/Playwright/Core/Artifact.cs | 12 +- src/Playwright/Core/Browser.cs | 28 +- src/Playwright/Core/BrowserContext.cs | 101 ++--- src/Playwright/Core/BrowserType.cs | 36 +- src/Playwright/Core/CDPSession.cs | 4 +- src/Playwright/Core/Dialog.cs | 4 +- src/Playwright/Core/ElementHandle.cs | 20 +- src/Playwright/Core/Frame.cs | 37 +- src/Playwright/Core/JSHandle.cs | 4 +- src/Playwright/Core/Locator.cs | 18 +- src/Playwright/Core/Mouse.cs | 4 +- src/Playwright/Core/Page.cs | 162 ++++--- src/Playwright/Core/PlaywrightImpl.cs | 2 +- src/Playwright/Core/Request.cs | 4 +- src/Playwright/Core/Response.cs | 14 +- src/Playwright/Core/Route.cs | 22 +- src/Playwright/Core/RouteHandler.cs | 16 +- src/Playwright/Core/Screencast.cs | 16 +- src/Playwright/Core/Selectors.cs | 19 +- src/Playwright/Core/Stream.cs | 4 +- src/Playwright/Core/Tracing.cs | 28 +- src/Playwright/Core/Waiter.cs | 30 +- src/Playwright/Core/WebMCP.cs | 8 +- src/Playwright/Core/WebSocketRoute.cs | 8 +- src/Playwright/Core/WebSocketRouteHandler.cs | 14 +- src/Playwright/Core/WebStorage.cs | 8 +- src/Playwright/Core/Worker.cs | 25 +- src/Playwright/Helpers/CopyOnWrite.cs | 52 +++ src/Playwright/Transport/ChannelOwner.cs | 77 ++-- src/Playwright/Transport/Connection.cs | 210 ++++++--- src/Playwright/Transport/Dispatcher.cs | 254 +++++++++++ src/Playwright/Transport/StdIOTransport.cs | 2 +- 43 files changed, 1762 insertions(+), 374 deletions(-) create mode 100644 docs/threading.md create mode 100644 src/Playwright.Tests/DispatcherTests.cs create mode 100644 src/Playwright.Tests/ThreadingModelTests.cs create mode 100644 src/Playwright/Helpers/CopyOnWrite.cs create mode 100644 src/Playwright/Transport/Dispatcher.cs diff --git a/CLAUDE.md b/CLAUDE.md index 87b3b0c85c..4381faef84 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,13 @@ The tests are NUnit tests in `src/Playwright.Tests/`, one class per upstream spe - New channel object types require: enum entry in `ChannelOwnerType.cs`, case in `Connection.cs`, initializer in `Transport/Protocol/Generated/`, and a `Core/` class. - Public APIs should use .NET standard types (e.g. `IAsyncDisposable`) not custom Playwright types. Internal helpers (e.g. `Disposable` class in `Core/`) stay internal. +### Threading model +The client mirrors the JS event loop: each connection has one dispatcher thread (`Transport/Dispatcher.cs`) that handles driver messages and runs all library code, one item at a time. Read [docs/threading.md](docs/threading.md) before changing code in `Core/` or `Transport/`; it has the model and the rules of thumb. The short version: +- A public async method is a single `SendMessageToServerAsync` call, or wraps its whole body in `WrapApiCallAsync(async () => ...)`. +- State is touched only on the dispatcher thread. A `TaskCompletionSource` is completed with `Dispatcher.Complete`/`Fail`, and code that awaited something else (a user callback, a timeout, I/O) calls `await _connection.Dispatcher.SwitchTo()` before it touches state. +- Lists that getters hand out are copy-on-write (`Helpers/CopyOnWrite.cs`), and nothing blocks on the dispatcher thread. +- `ThreadingModelTests.cs` and `DispatcherTests.cs` pin the rules. + ## Commit Convention Before committing, run `dotnet format ./src/ -v:diag` and fix style issues. diff --git a/docs/threading.md b/docs/threading.md new file mode 100644 index 0000000000..64777d1dfb --- /dev/null +++ b/docs/threading.md @@ -0,0 +1,56 @@ +# Threading model + +The JS client runs on one event loop: a driver message is handled synchronously, and the promise continuations it resolves run as microtasks before the next message. Library code between two awaits can therefore check a state and subscribe to an event without anything happening in between. The .NET client keeps the JS method bodies, so it restores that property with one **dispatcher thread** per connection (`src/Playwright/Transport/Dispatcher.cs`). + +## How it works + +- **One thread, one item at a time.** The dispatcher thread runs driver messages (`Connection.Dispatch`) and all library code. The transport reads and parses on the thread pool and posts each message to the dispatcher. A connection that `BrowserType.ConnectAsync` creates shares the dispatcher of the connection that owns its pipe. +- **API calls hop on and off.** `Connection.WrapApiCallAsync` posts the body of a public call to the dispatcher, and returns the result on the thread pool. The caller's continuation never runs on the dispatcher thread, and a `SynchronizationContext` that the caller captured is honored as with any other `Task`. +- **Microtasks.** A `TaskCompletionSource` that library code awaits is completed with `Dispatcher.Complete` or `Dispatcher.Fail`: after the current item, before the next one. The awaiting code continues inline on the dispatcher thread, because every library `await` uses `ConfigureAwait(false)` and the thread has no `SynchronizationContext`. So a call resumes once the message that completed it has been handled in full: `Page.Load` has fired when `WaitForLoadStateAsync` returns. +- **Event accessors update a snapshot.** `+=` and `-=` run on the caller's thread and replace the handler list atomically (`ChannelOwner.UpdateEventHandler`), so the dispatcher thread raises either the old list or the new one. For the events that the server sends only while somebody listens, the dispatcher thread alone tells the server, from the handlers as they are when it runs. +- **Getters read snapshots.** Synchronous getters run on the caller's thread. Scalars (`Url`, `IsClosed`) are single reads. Lists (`Pages`, `Frames`, `ChildFrames`, `Workers`, `Contexts`) are copy-on-write through `Helpers/CopyOnWrite.cs`: a published list is never mutated. + +Where user code runs: + +| User code | Thread | +|---|---| +| Event handler, route handler, binding, locator handler: the synchronous part | dispatcher | +| The same handler after it awaited a Playwright call or anything else | thread pool | +| Predicate of a wait (`WaitForRequestAsync(r => ...)`, `WaitForURLAsync(url => ...)`) | dispatcher | +| Action of `RunAndWaitFor*Async` | thread pool | +| Code after `await` of any Playwright call | thread pool, or the caller's `SynchronizationContext` | + +## Rules of thumb + +1. **Wrap public async methods.** A public async method is either a single `SendMessageToServerAsync` call plus parsing of its result, or its whole body is inside `WrapApiCallAsync(async () => ...)`. Private helpers that a public method delegates to are wrapped as well; a nested wrap is inline and free. Put the wrap on the signature line and keep the body at the indentation of a method body, so that wrapping a method does not re-indent it: + + ```csharp + public Task CloseAsync(PageCloseOptions? options = default) => WrapApiCallAsync(async () => + { + ... + }); + ``` +2. **Touch state only on the dispatcher thread.** Fields such as `_routes`, `_loadStates`, `_pages` and `Bindings` are read and written there. Put `_connection.Dispatcher.AssertCurrent()` next to new state access; it fails in Debug builds, which is what the tests run. +3. **Complete through the dispatcher.** Use `Dispatcher.Complete` and `Dispatcher.Fail`, never `SetResult` or `SetException`, on a source that library code awaits. Do not create such a source with `RunContinuationsAsynchronously`. +4. **Switch back after a foreign await.** `await _connection.Dispatcher.SwitchTo()` before touching state after awaiting something that the dispatcher did not complete: + - a user callback (put the switch in a `finally`, the callback may throw), + - a timer or a timeout (`Task.Delay`, `WithTimeout`), file or pipe I/O, + - a `Task` that another call started and cached, + - any library call in a flow that a message started (`OnRouteAsync`, `BindingCall.CallAsync`). Such a flow is not inside an API call, so the calls it makes are outermost and return on the thread pool. +5. **Synchronous API never waits for the dispatcher.** It may be called from a handler that runs on it. Read a snapshot, or hand work to the dispatcher with `Dispatcher.RunOrPost`: it is ordered with the caller's other calls. +6. **Hand out copy-on-write lists.** A collection that a getter returns, or that synchronous API enumerates, is replaced with `CopyOnWrite.Add` and `CopyOnWrite.Remove`, never mutated in place. +7. **Never block the dispatcher thread.** No `.Result`, `.Wait()` or `GetAwaiter().GetResult()` on a Playwright call in library code that may run there. +8. **Keep `ConfigureAwait(false)` everywhere**, and pass `TaskScheduler.Default` to `ContinueWith` and `StartNew`. +9. **Clear the API zone around user code.** A call is nested, and stays on the dispatcher thread, when `Connection.ApiZone` is set. User code that runs inside a wrapped body must not inherit it; see `Waiter.CancelWaitOnExceptionAsync`. + +## What users can rely on + +- Events that the driver sent before a reply have been handled when the call returns. +- Blocking is safe everywhere except in the synchronous part of a handler: a synchronous wait on a Playwright call there stalls the connection until it times out. +- A handler that throws closes the connection, as before. +- `+=` and `-=` on an event take effect at once, as with any C# event. After `-=` has returned, the handler is not called for an event that is raised later; only an event that is being raised on the dispatcher thread at that very moment may still reach it. +- Arguments are read when the call runs on the dispatcher thread, shortly after it was made. Do not mutate an options object or a buffer that a call in flight still uses. + +## Tests + +`DispatcherTests.cs` pins the dispatcher itself. `ThreadingModelTests.cs` pins the rules above end to end; add a case there when a rule changes. diff --git a/src/Playwright.Tests/BrowserContextBasicTests.cs b/src/Playwright.Tests/BrowserContextBasicTests.cs index c0f352b7e2..7df78b389b 100644 --- a/src/Playwright.Tests/BrowserContextBasicTests.cs +++ b/src/Playwright.Tests/BrowserContextBasicTests.cs @@ -312,4 +312,18 @@ public async Task ShouldEmulateNavigatorOnLine() await context.SetOfflineAsync(false); Assert.True(await page.EvaluateAsync("() => window.navigator.onLine")); } + + [PlaywrightTest] + public async Task ShouldAllowOpeningPagesWhileEnumeratingPages() + { + // Pages used to hand out the live list, so a page opened from inside the loop made the + // next MoveNext throw "Collection was modified". + await using var context = await Browser.NewContextAsync(); + await context.NewPageAsync(); + foreach (var page in context.Pages) + { + await context.NewPageAsync(); + } + Assert.AreEqual(2, context.Pages.Count); + } } diff --git a/src/Playwright.Tests/DispatcherTests.cs b/src/Playwright.Tests/DispatcherTests.cs new file mode 100644 index 0000000000..a9e47ccecf --- /dev/null +++ b/src/Playwright.Tests/DispatcherTests.cs @@ -0,0 +1,247 @@ +/* + * MIT License + * + * Copyright (c) Microsoft Corporation. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and / or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +using Microsoft.Playwright.Transport; + +namespace Microsoft.Playwright.Tests; + +/// +/// The dispatcher is the thread on which a connection handles driver messages and runs library +/// code; see . These tests pin the three rules the rest of the client +/// depends on: items run on that thread in order, a completion source completed through the +/// dispatcher resumes its awaiter after the current item and on the dispatcher thread, and the +/// two hops (onto the thread and off it) land where they say. +/// +public class DispatcherTests +{ + private Dispatcher _dispatcher = null!; + + private static bool OnDispatcherThread => Thread.CurrentThread.Name == Dispatcher.ThreadName; + + [SetUp] + public void SetUp() => _dispatcher = new Dispatcher(); + + [TearDown] + public void TearDown() => _dispatcher.Dispose(); + + [Test] + public async Task PostRunsItemsOnTheDispatcherThreadInOrder() + { + var log = new List(); + var done = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + for (int i = 0; i < 100; i++) + { + int n = i; + _dispatcher.Post(() => + { + Assert.True(_dispatcher.IsCurrent); + Assert.True(OnDispatcherThread); + log.Add($"item{n}"); + }); + } + _dispatcher.Post(() => done.SetResult(true)); + await done.Task; + Assert.AreEqual(Enumerable.Range(0, 100).Select(i => $"item{i}").ToList(), log); + } + + [Test] + public async Task SwitchToLandsOnTheDispatcherThreadAndIsSynchronousThere() + { + Assert.False(_dispatcher.IsCurrent); + Assert.False(_dispatcher.SwitchTo().IsCompleted); + await _dispatcher.SwitchTo(); + Assert.True(_dispatcher.IsCurrent); + Assert.True(_dispatcher.SwitchTo().IsCompleted); + // A second hop from the dispatcher thread is a no-op. + await _dispatcher.SwitchTo(); + Assert.True(_dispatcher.IsCurrent); + await Dispatcher.ExitToThreadPool(); + Assert.False(_dispatcher.IsCurrent); + } + + [Test] + public async Task SwitchToFlowsTheExecutionContext() + { + var local = new AsyncLocal(); + local.Value = "from the caller"; + await _dispatcher.SwitchTo(); + Assert.AreEqual("from the caller", local.Value); + await Dispatcher.ExitToThreadPool(); + Assert.AreEqual("from the caller", local.Value); + } + + [Test] + public async Task CompleteRunsTheContinuationAfterTheCurrentItemOnTheDispatcherThread() + { + // The library's await: ConfigureAwait(false) on a source that the dispatcher completes. + var tcs = new TaskCompletionSource(); + var itemFinished = false; + var continuation = Task.Run(async () => + { + var value = await tcs.Task.ConfigureAwait(false); + Assert.True(itemFinished, "the continuation must run after the item that completed the source"); + Assert.True(_dispatcher.IsCurrent, "the continuation must run inline on the dispatcher thread"); + return value; + }); + // Give the awaiter time to subscribe before the source is completed. + await Task.Delay(50); + _dispatcher.Post(() => + { + _dispatcher.Complete(tcs, 42); + Assert.False(tcs.Task.IsCompleted, "completion is deferred until the item has finished"); + itemFinished = true; + }); + Assert.AreEqual(42, await continuation); + } + + [Test] + public async Task MicrotasksRunBeforeTheNextItem() + { + var log = new List(); + var tcs1 = new TaskCompletionSource(); + var tcs2 = new TaskCompletionSource(); + var done = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + // Subscribe from the dispatcher thread, so that the continuations run inline there. + await _dispatcher.SwitchTo(); + _ = tcs1.Task.ContinueWith( + _ => + { + log.Add("continuation1"); + // A microtask queued by a microtask still runs before the next item. + _dispatcher.Complete(tcs2, true); + }, + TaskContinuationOptions.ExecuteSynchronously); + _ = tcs2.Task.ContinueWith(_ => log.Add("continuation2"), TaskContinuationOptions.ExecuteSynchronously); + _dispatcher.Post(() => + { + log.Add("item1"); + _dispatcher.Complete(tcs1, true); + log.Add("item1 end"); + }); + _dispatcher.Post(() => log.Add("item2")); + _dispatcher.Post(() => done.SetResult(true)); + await Dispatcher.ExitToThreadPool(); + await done.Task; + Assert.AreEqual(new[] { "item1", "item1 end", "continuation1", "continuation2", "item2" }, log); + } + + [Test] + public async Task CompleteFromAnotherThreadResumesTheAwaiterOnTheDispatcherThread() + { + var tcs = new TaskCompletionSource(); + var continuation = Task.Run(async () => + { + await tcs.Task.ConfigureAwait(false); + return _dispatcher.IsCurrent; + }); + await Task.Delay(50); + Assert.False(_dispatcher.IsCurrent); + _dispatcher.Complete(tcs, true); + Assert.True(await continuation); + } + + [Test] + public async Task FailFaultsTheSourceAfterTheCurrentItem() + { + var tcs = new TaskCompletionSource(); + var continuation = Task.Run(async () => + { + try + { + await tcs.Task.ConfigureAwait(false); + return null; + } + catch (InvalidOperationException ex) + { + Assert.True(_dispatcher.IsCurrent); + return ex.Message; + } + }); + await Task.Delay(50); + _dispatcher.Post(() => _dispatcher.Fail(tcs, new InvalidOperationException("boom"))); + Assert.AreEqual("boom", await continuation); + } + + [Test] + public async Task InlineContinuationChainStaysOnTheDispatcherThread() + { + // Nested library methods awaiting each other with ConfigureAwait(false): every level + // resumes on the dispatcher thread when the innermost source is completed there. + var tcs = new TaskCompletionSource(); + var threads = new List(); + + async Task Level(int depth) + { + if (depth == 0) + { + await tcs.Task.ConfigureAwait(false); + } + else + { + await Level(depth - 1).ConfigureAwait(false); + } + threads.Add(_dispatcher.IsCurrent); + } + + var chain = Task.Run(() => Level(20)); + await Task.Delay(50); + _dispatcher.Post(() => _dispatcher.Complete(tcs, true)); + await chain; + Assert.AreEqual(21, threads.Count); + Assert.True(threads.All(onDispatcher => onDispatcher)); + } + + [Test] + public async Task ExitToThreadPoolNeverCompletesSynchronously() + { + Assert.False(Dispatcher.ExitToThreadPool().IsCompleted); + var thread = Thread.CurrentThread; + await Dispatcher.ExitToThreadPool(); + Assert.True(Thread.CurrentThread.IsThreadPoolThread); + Assert.False(_dispatcher.IsCurrent); + } + + [Test] + public async Task PostAfterDisposeStillRunsTheAction() + { + _dispatcher.Dispose(); + var ran = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _dispatcher.Post(() => ran.SetResult(true)); + Assert.True(await ran.Task.WaitAsync(TimeSpan.FromSeconds(5))); + // The hop still resolves, on some thread, so that a call made after close can throw. + await _dispatcher.SwitchTo(); + } + + [Test] + public async Task UnhandledExceptionInAnItemIsReportedAndTheThreadSurvives() + { + Exception reported = null; + _dispatcher.UnhandledException += (_, ex) => reported = ex; + var done = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _dispatcher.Post(() => throw new InvalidOperationException("boom")); + _dispatcher.Post(() => done.SetResult(true)); + await done.Task; + Assert.AreEqual("boom", reported?.Message); + } +} diff --git a/src/Playwright.Tests/FrameHierarchyTests.cs b/src/Playwright.Tests/FrameHierarchyTests.cs index eeb6e8dcb6..7bb452cba6 100644 --- a/src/Playwright.Tests/FrameHierarchyTests.cs +++ b/src/Playwright.Tests/FrameHierarchyTests.cs @@ -209,4 +209,24 @@ await Page.EvaluateAsync(@"() => { Assert.False(frame2.IsDetached); Assert.That(frame1, Is.Not.EqualTo(frame2)); } + + [PlaywrightTest] + public async Task ShouldAllowAttachingFramesWhileEnumeratingChildFrames() + { + // ChildFrames used to hand out the live list, so a frame attached from inside the loop + // made the next MoveNext throw "Collection was modified". + await Page.GotoAsync(Server.EmptyPage); + await FrameUtils.AttachFrameAsync(Page, "frame1", Server.EmptyPage); + foreach (var frame in Page.MainFrame.ChildFrames) + { + await FrameUtils.AttachFrameAsync(Page, "frame2", Server.EmptyPage); + } + Assert.AreEqual(2, Page.MainFrame.ChildFrames.Count); + int next = 3; + foreach (var frame in Page.Frames) + { + await FrameUtils.AttachFrameAsync(Page, $"frame{next++}", Server.EmptyPage); + } + Assert.AreEqual(6, Page.Frames.Count); + } } diff --git a/src/Playwright.Tests/PageRunAndWaitForResponseTests.cs b/src/Playwright.Tests/PageRunAndWaitForResponseTests.cs index 89750a4b93..103a3d2187 100644 --- a/src/Playwright.Tests/PageRunAndWaitForResponseTests.cs +++ b/src/Playwright.Tests/PageRunAndWaitForResponseTests.cs @@ -109,4 +109,18 @@ public async Task ShouldWorkWithUrlMatch() Assert.AreEqual(Server.Prefix + "/digits/1.png", response.Url); } + + // See https://github.com/microsoft/playwright-dotnet/pull/3256 + [PlaywrightTest] + public async Task ShouldNotDeadlockWhenBlockingAfterTheWait() + { + // The code after the wait used to run inside the handler of the `response` event, on the + // thread that delivers messages, so a synchronous wait there never got its reply. + await Page.GotoAsync(Server.EmptyPage); + var response = await Page.RunAndWaitForResponseAsync(() => Page.EvaluateAsync(@"() => { + fetch('/digits/1.png'); + }"), Server.Prefix + "/digits/1.png"); + Assert.AreEqual(Server.Prefix + "/digits/1.png", response.Url); + Assert.True(Task.Run(() => Page.GotoAsync(Server.EmptyPage)).Wait(10_000)); + } } diff --git a/src/Playwright.Tests/PageWaitForLoadStateTests.cs b/src/Playwright.Tests/PageWaitForLoadStateTests.cs index 4b6d8cd404..a533d47faa 100644 --- a/src/Playwright.Tests/PageWaitForLoadStateTests.cs +++ b/src/Playwright.Tests/PageWaitForLoadStateTests.cs @@ -138,4 +138,32 @@ await Page.RouteAsync(Server.Prefix + "/one-style.css", async (route) => requestTask.TrySetResult(true); await loadTask; } + + // See https://github.com/microsoft/playwright-dotnet/issues/3399 + [PlaywrightTest] + public async Task ShouldObserveLoadStateFiredWhileTheWaitIsBeingSetUp() + { + // The `load` state arrives while another thread is calling WaitForLoadStateAsync, a + // random moment later each time. The wait checks the recorded states and subscribes to + // the next one in one step, so the state lands either before the check or after the + // subscription, never in between, where it used to be lost until the timeout. + var frame = (Core.Frame)Page.MainFrame; + var dispatcher = frame._connection.Dispatcher; + await Page.GotoAsync(Server.EmptyPage); + + var random = new Random(3399); + for (int i = 0; i < 200; i++) + { + // Forget the recorded state, so that the wait has to observe the event. + dispatcher.Post(() => frame.OnLoadState(null, WaitUntilState.Load)); + int spin = random.Next(0, 20000); + var fire = Task.Run(() => + { + Thread.SpinWait(spin); + dispatcher.Post(() => frame.OnLoadState(WaitUntilState.Load, null)); + }); + await frame.WaitForLoadStateAsync(LoadState.Load, new() { Timeout = 5000 }); + await fire; + } + } } diff --git a/src/Playwright.Tests/PageWaitForUrlTests.cs b/src/Playwright.Tests/PageWaitForUrlTests.cs index 786a6bd9ff..ee1b9f33b9 100644 --- a/src/Playwright.Tests/PageWaitForUrlTests.cs +++ b/src/Playwright.Tests/PageWaitForUrlTests.cs @@ -22,6 +22,7 @@ * SOFTWARE. */ +using System.Text.Json; using System.Text.RegularExpressions; namespace Microsoft.Playwright.Tests; @@ -161,4 +162,78 @@ await TaskUtils.WhenAll( frame.EvaluateAsync("url => window.location.href = url", Server.Prefix + "/grid.html")); ; } + + // See https://github.com/microsoft/playwright-dotnet/issues/3364 + [PlaywrightTest] + public async Task ShouldNotMissNavigationCommittedRightAfterTheCall() + { + // Page A navigates to B a few milliseconds after it loads. The wait must see that + // navigation even when it commits right after WaitForURLAsync was called: the check of + // the current URL and the subscription to the next navigation are one step. + Server.SetRoute("/a.html", context => + { + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync(""); + }); + Server.SetRoute("/b.html", context => + { + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync("

b

"); + }); + for (int i = 0; i < 10; i++) + { + await Page.GotoAsync(Server.Prefix + "/a.html"); + await Page.WaitForURLAsync("**/b.html", new() { Timeout = 5000 }); + } + } + + // See https://github.com/microsoft/playwright-dotnet/issues/3399 + [PlaywrightTest] + public async Task ShouldNotMissLoadThatFiresRightAfterTheCall() + { + // Each page links to the next one and loads a script. The response arrives first, and + // `load` follows a few milliseconds later: right when WaitForURLAsync checks the load + // state of a URL that already matches and subscribes to the next one. + Server.SetRoute("/slow-script.js", async context => + { + context.Response.ContentType = "application/javascript"; + await Task.Delay(5); + await context.Response.WriteAsync("window.resourceLoaded = true;"); + }); + for (int i = 0; i < 20; i++) + { + int next = i + 1; + Server.SetRoute($"/p/{i}", context => + { + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync($"next"); + }); + } + await Page.GotoAsync(Server.Prefix + "/p/0"); + for (int i = 1; i < 20; i++) + { + var response = await Page.RunAndWaitForResponseAsync( + () => Page.Locator("a").ClickAsync(), + r => r.Request.IsNavigationRequest && r.Status == 200); + await Page.WaitForURLAsync(response.Url, new() { WaitUntil = WaitUntilState.Load, Timeout = 5000 }); + Assert.True(await Page.EvaluateAsync("window.resourceLoaded === true")); + } + } + + [PlaywrightTest] + public async Task ShouldNotBlockTheDispatcherWhileMatchingTheUrl() + { + // The predicate is user code and runs on the dispatcher thread, like an event handler. + // Awaiting a call from there is fine; this test pins that the wait does not hold anything + // that the reply to that call would need. + await Page.GotoAsync(Server.EmptyPage); + Task evaluate = null; + await Page.WaitForURLAsync(url => + { + evaluate ??= Page.EvaluateAsync("() => { document.body.appendChild(document.createElement('iframe')); }"); + return true; + }); + await evaluate!; + Assert.AreEqual(2, Page.Frames.Count); + } } diff --git a/src/Playwright.Tests/PlaywrightDisposeTests.cs b/src/Playwright.Tests/PlaywrightDisposeTests.cs index 1617fe962e..a8b1a48392 100644 --- a/src/Playwright.Tests/PlaywrightDisposeTests.cs +++ b/src/Playwright.Tests/PlaywrightDisposeTests.cs @@ -75,4 +75,23 @@ public async Task ShouldDisposeFromEventHandlerWhileDriverIsSendingEvents() // Many times more than a pipe holds, and it never stops. private const string ConsoleFlood = "() => { setInterval(() => { for (let i = 0; i < 200; i++) console.log('x'.repeat(20000)); }, 0); }"; + + // See https://github.com/microsoft/playwright-dotnet/issues/3446 + [PlaywrightTest] + public async Task ShouldFailCallsInFlightWhenPlaywrightIsDisposed() + { + var localPlaywright = await Microsoft.Playwright.Playwright.CreateAsync(); + var browser = await localPlaywright[BrowserName].LaunchAsync(); + var page = await browser.NewPageAsync(); + + // Calls that never get a reply. They are sent in order, before the call that follows. + var calls = Enumerable.Range(0, 20).Select(_ => page.EvaluateAsync("() => new Promise(() => {})")).ToArray(); + await page.EvaluateAsync("1"); + + await Task.Run(() => localPlaywright.Dispose()); + foreach (var call in calls) + { + await PlaywrightAssert.ThrowsAsync(() => call.WaitAsync(TimeSpan.FromSeconds(10))); + } + } } diff --git a/src/Playwright.Tests/ThreadingModelTests.cs b/src/Playwright.Tests/ThreadingModelTests.cs new file mode 100644 index 0000000000..68f6e21b3f --- /dev/null +++ b/src/Playwright.Tests/ThreadingModelTests.cs @@ -0,0 +1,397 @@ +/* + * MIT License + * + * Copyright (c) Microsoft Corporation. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and / or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +using Microsoft.Playwright.Transport; + +namespace Microsoft.Playwright.Tests; + +/// +/// Pins the client's threading model; see "Threading model" in CLAUDE.md. Driver messages and +/// library code run on one dispatcher thread per connection, event handlers run on it too, an +/// awaited call resumes once the message that completed it has been handled in full, and the +/// caller's continuation never runs on the dispatcher thread. +/// +public class ThreadingModelTests : PageTestEx +{ + private static bool OnDispatcherThread => Thread.CurrentThread.Name == Dispatcher.ThreadName; + + // Navigates to a page that commits at once (Firefox and WebKit need a few KB before they + // commit) and finishes loading only when the returned action is invoked, so that a + // WaitForLoadStateAsync started before that has to wait for the `load` event. + private async Task GotoPageThatLoadsOnDemandAsync() + { + var finishBody = new TaskCompletionSource(); + Server.SetRoute("/slow.html", async ctx => + { + ctx.Response.ContentType = "text/html; charset=utf-8"; + await ctx.Response.WriteAsync(""); + await ctx.Response.Body.FlushAsync(); + await finishBody.Task; + await ctx.Response.WriteAsync(""); + }); + await Page.GotoAsync(Server.Prefix + "/slow.html", new() { WaitUntil = WaitUntilState.Commit }); + return () => finishBody.SetResult(true); + } + + [PlaywrightTest] + public async Task EventsBeforeTheReplyAreHandledBeforeTheMethodReturns() + { + // Messages are handled one at a time and a reply only queues the continuation, + // so everything the `load` event did is visible when GotoAsync returns. + int loads = 0; + Page.Load += (_, _) => Interlocked.Increment(ref loads); + for (int i = 1; i <= 20; i++) + { + await Page.GotoAsync(Server.EmptyPage); + Assert.AreEqual(i, Volatile.Read(ref loads)); + } + } + + [PlaywrightTest] + public async Task PageLoadHasFiredWhenWaitForLoadStateReturns() + { + // Frame.OnLoadState raises Frame.LoadState, which completes the wait, before Page.Load. + // The continuation runs after the whole message has been handled, as a JS microtask would. + bool loadFired = false; + Page.Load += (_, _) => loadFired = true; + var finishLoading = await GotoPageThatLoadsOnDemandAsync(); + var load = Page.WaitForLoadStateAsync(LoadState.Load); + finishLoading(); + await load; + Assert.True(loadFired); + } + + [PlaywrightTest] + public async Task ApiCallsResumeOffTheDispatcherThread() + { + // Whether a call completes with a reply or with an event, the caller continues on the + // thread pool, so that it may block there. + await Page.GotoAsync(Server.EmptyPage); + Assert.False(OnDispatcherThread); + + var finishLoading = await GotoPageThatLoadsOnDemandAsync(); + var load = Page.WaitForLoadStateAsync(LoadState.Load); + finishLoading(); + await load; + Assert.False(OnDispatcherThread); + + await Page.RunAndWaitForResponseAsync(() => Page.EvaluateAsync("fetch('/empty.html')"), "**/empty.html"); + Assert.False(OnDispatcherThread); + + await Page.CloseAsync(); + Assert.False(OnDispatcherThread); + } + + [PlaywrightTest] + public async Task BlockingAfterAnEventDrivenWaitDoesNotStallTheConnection() + { + var finishLoading = await GotoPageThatLoadsOnDemandAsync(); + var load = Page.WaitForLoadStateAsync(LoadState.Load); + finishLoading(); + await load; + var evaluate = Page.EvaluateAsync("1 + 1"); + Assert.True(evaluate.Wait(5000)); + Assert.AreEqual(2, await evaluate); + } + + [PlaywrightTest] + public async Task EventHandlersRunOnTheDispatcherThread() + { + bool? onDispatcher = null; + Page.Console += (_, _) => onDispatcher = OnDispatcherThread; + await Page.EvaluateAsync("console.log('hi')"); + Assert.True(onDispatcher); + } + + [PlaywrightTest] + public async Task AwaitingInsideAnEventHandlerResumesOffTheDispatcherThread() + { + // Only the synchronous part of a handler runs on the dispatcher thread. A call that the + // handler awaits returns on the thread pool, so the handler may block from there on. + var result = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + Page.Console += async (_, _) => + { + try + { + Assert.True(OnDispatcherThread); + var value = await Page.EvaluateAsync("1 + 1"); + Assert.False(OnDispatcherThread); + var blocking = Page.EvaluateAsync("2 + 2"); + Assert.True(blocking.Wait(5000)); + result.SetResult(value + blocking.Result); + } + catch (Exception ex) + { + result.SetException(ex); + } + }; + await Page.EvaluateAsync("console.log('hi')"); + Assert.AreEqual(6, await result.Task); + } + + [PlaywrightTest] + public async Task BlockingInAnEventHandlerStallsTheConnection() + { + // The one rule for users: a synchronous wait on a Playwright call in the synchronous + // part of a handler blocks the thread that would deliver its reply. + bool completed = true; + Page.Console += (_, _) => completed = Page.EvaluateAsync("1 + 1").Wait(1000); + await Page.EvaluateAsync("console.log('hi')"); + Assert.False(completed); + } + + [PlaywrightTest] + public async Task RunAndWaitForActionRunsOffTheDispatcherThread() + { + bool? onDispatcher = null; + await Page.RunAndWaitForConsoleMessageAsync(async () => + { + onDispatcher = OnDispatcherThread; + await Page.EvaluateAsync("console.log('hi')"); + onDispatcher |= OnDispatcherThread; + }); + Assert.False(onDispatcher); + } + + [PlaywrightTest] + public async Task SubscribingRightAfterStartingAWaitKeepsBothHandlers() + { + // The wait subscribes on the dispatcher thread after the call has returned, and the + // caller subscribes to the same event on its own thread in the meantime. The handler + // list is updated atomically, so neither subscription overwrites the other. + await Page.GotoAsync(Server.EmptyPage); + for (int i = 0; i < 100; i++) + { + int seen = 0; + EventHandler handler = (_, _) => Interlocked.Increment(ref seen); + var wait = Page.WaitForRequestAsync($"**/empty.html?{i}", new() { Timeout = 5000 }); + Page.Request += handler; + await Page.EvaluateAsync("i => fetch('/empty.html?' + i).then(r => r.text())", i); + await wait; + Assert.AreEqual(1, Volatile.Read(ref seen), $"iteration {i}"); + Page.Request -= handler; + } + } + + [PlaywrightTest] + public async Task UnsubscribingRightAfterStartingAWaitKeepsTheWait() + { + // Removing the last handler disables the event on the server, and the wait that was + // started just before needs it enabled. Only the dispatcher thread tells the server, + // from the handlers as they are at that point, so the event stays enabled. + await Page.GotoAsync(Server.EmptyPage); + for (int i = 0; i < 100; i++) + { + EventHandler handler = (_, _) => { }; + Page.Request += handler; + var wait = Page.WaitForRequestAsync($"**/empty.html?{i}", new() { Timeout = 5000 }); + Page.Request -= handler; + await Page.EvaluateAsync("i => fetch('/empty.html?' + i).then(r => r.text())", i); + await wait; + } + } + + [PlaywrightTest] + public async Task RemovedHandlerIsNotCalledForEventsThatAreAlreadyQueued() + { + // Removing a handler takes effect at once, as with any C# event. Here the dispatcher + // thread is held inside a `request` handler while the page logs to the console, so the + // `console` messages wait in its queue. The handler removed meanwhile must not get them. + await Page.GotoAsync(Server.EmptyPage); + int calls = 0; + EventHandler removed = (_, _) => Interlocked.Increment(ref calls); + using var entered = new ManualResetEventSlim(); + using var release = new ManualResetEventSlim(); + Page.Request += (_, _) => + { + entered.Set(); + release.Wait(TimeSpan.FromSeconds(10)); + }; + Page.Console += removed; + + var evaluate = Page.EvaluateAsync(@"async () => { + await fetch('/empty.html').then(r => r.text()); + for (let i = 0; i < 5; i++) + console.log(i); + }"); + Assert.True(entered.Wait(TimeSpan.FromSeconds(10))); + // Let the page finish and its console messages reach the queue. + await Task.Delay(500); + Page.Console -= removed; + release.Set(); + await evaluate; + + // A reply follows the events that preceded it, so all of them have been handled. + Assert.AreEqual(0, Volatile.Read(ref calls)); + } + + [PlaywrightTest] + public async Task HandlersAddedAndRemovedFromManyThreadsAreAllKept() + { + // The handler list is updated atomically: no thread overwrites the update of another, + // and the server is told to send the event exactly while there are handlers. + const int threads = 8; + const int perThread = 50; + int calls = 0; + var handlers = Enumerable.Range(0, threads * perThread) + .Select(_ => (EventHandler)((_, _) => Interlocked.Increment(ref calls))) + .ToArray(); + + await Task.WhenAll(Enumerable.Range(0, threads).Select(t => Task.Run(() => + { + for (int i = 0; i < perThread; i++) + { + Page.Console += handlers[(t * perThread) + i]; + } + }))); + await Page.EvaluateAsync("console.log('hi')"); + Assert.AreEqual(threads * perThread, Volatile.Read(ref calls)); + + await Task.WhenAll(Enumerable.Range(0, threads).Select(t => Task.Run(() => + { + for (int i = 0; i < perThread; i++) + { + Page.Console -= handlers[(t * perThread) + i]; + } + }))); + await Page.EvaluateAsync("console.log('hi')"); + Assert.AreEqual(threads * perThread, Volatile.Read(ref calls)); + + // The event still works after its last handler was removed. + var message = await Page.RunAndWaitForConsoleMessageAsync(() => Page.EvaluateAsync("console.log('again')")); + Assert.AreEqual("again", message.Text); + } + + [PlaywrightTest] + public async Task RunAndWaitForActionThatThrowsAfterTheEventDoesNotBreakTheConnection() + { + // The event arrives first, and the action then fails on the thread pool. The waiter is + // cleaned up on the dispatcher thread, and the page keeps working. + var exception = await PlaywrightAssert.ThrowsAsync(() => Page.RunAndWaitForConsoleMessageAsync(async () => + { + await Page.EvaluateAsync("console.log('hi')"); + await Task.Delay(50); + throw new InvalidOperationException("boom"); + })); + Assert.AreEqual("boom", exception.Message); + Assert.AreEqual(2, await Page.EvaluateAsync("1 + 1")); + } + + [PlaywrightTest] + public async Task RouteHandlerMayResumeOnAnotherThread() + { + // A handler that awaits something foreign resumes on the thread pool; the library + // switches back before it touches its state, and the second handler still runs. + var log = new List(); + await Page.RouteAsync("**/empty.html", async route => + { + await Task.Delay(10); + log.Add("second:" + OnDispatcherThread); + await route.FulfillAsync(new() { ContentType = "text/html", Body = "routed" }); + }); + await Page.RouteAsync("**/empty.html", async route => + { + await Task.Delay(10); + log.Add("first:" + OnDispatcherThread); + await route.FallbackAsync(); + }); + await Page.GotoAsync(Server.EmptyPage); + Assert.AreEqual("routed", await Page.TitleAsync()); + Assert.AreEqual(new[] { "first:False", "second:False" }, log); + } + + [PlaywrightTest] + public async Task CallerSynchronizationContextIsHonored() + { + // The hop off the dispatcher completes on the thread pool, so an awaiter that captured + // a SynchronizationContext is posted back to it, as with any other Task. + var context = new CountingSynchronizationContext(); + var previous = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(context); + try + { + await Page.EvaluateAsync("1"); + Assert.AreEqual(1, context.Posts); + Assert.False(OnDispatcherThread); + } + finally + { + SynchronizationContext.SetSynchronizationContext(previous); + } + } + + [PlaywrightTest] + public async Task ConcurrentCallsAndReadsDoNotCorruptState() + { + // Pages opened from many threads while another thread enumerates Context.Pages. + using var cts = new CancellationTokenSource(); + var reader = Task.Run(() => + { + int seen = 0; + while (!cts.Token.IsCancellationRequested) + { + foreach (var page in Context.Pages) + { + seen++; + } + } + return seen; + }); + var pages = await Task.WhenAll(Enumerable.Range(0, 10).Select(_ => Task.Run(() => Context.NewPageAsync()))); + Assert.AreEqual(11, Context.Pages.Count); + await Task.WhenAll(pages.Select(page => Task.Run(() => page.CloseAsync()))); + Assert.AreEqual(1, Context.Pages.Count); + cts.Cancel(); + Assert.Greater(await reader, 0); + } + + [PlaywrightTest] + public async Task ThrowingInAnEventHandlerClosesTheConnection() + { + // Handlers run inside message dispatch, whose catch-all closes the connection, so one + // throwing handler takes down every page and browser of the IPlaywright instance. + // Uses its own instance so that the shared one survives; the driver exits with it. + using var playwright = await Microsoft.Playwright.Playwright.CreateAsync(); + var browser = await playwright[BrowserName].LaunchAsync(await LaunchOptionsAsync()); + var page = await browser.NewPageAsync(); + page.Console += (_, _) => throw new InvalidOperationException("boom"); + + var exception = Assert.ThrowsAsync(() => page.EvaluateAsync("console.log('hi')")); + StringAssert.Contains("boom", exception.Message); + Assert.ThrowsAsync(() => browser.NewPageAsync()); + } + + private sealed class CountingSynchronizationContext : SynchronizationContext + { + private int _posts; + + public int Posts => Volatile.Read(ref _posts); + + public override void Post(SendOrPostCallback d, object state) + { + Interlocked.Increment(ref _posts); + ThreadPool.QueueUserWorkItem(_ => d(state)); + } + } +} diff --git a/src/Playwright/Core/APIRequestContext.cs b/src/Playwright/Core/APIRequestContext.cs index 3e5b8f4e62..7c441afddf 100644 --- a/src/Playwright/Core/APIRequestContext.cs +++ b/src/Playwright/Core/APIRequestContext.cs @@ -55,7 +55,7 @@ public async ValueTask DisposeAsync() await DisposeAsync(null).ConfigureAwait(false); } - internal async ValueTask DisposeAsync(string? reason) + internal ValueTask DisposeAsync(string? reason) => new(WrapApiCallAsync(async () => { _closeReason = reason; try @@ -70,7 +70,7 @@ internal async ValueTask DisposeAsync(string? reason) // Swallow exception } _tracing.ResetStackCounter(); - } + })); [MethodImpl(MethodImplOptions.NoInlining)] public Task> CookiesAsync() => CookiesAsync(Array.Empty()); @@ -95,7 +95,7 @@ public Task AddCookiesAsync(IEnumerable cookies) => SendMessageToServerA }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task ClearCookiesAsync(APIRequestContextClearCookiesOptions? options = default) + public Task ClearCookiesAsync(APIRequestContextClearCookiesOptions? options = default) => WrapApiCallAsync(async () => { var @params = BrowserContext.ToClearCookiesParams( options?.Name ?? options?.NameString, @@ -106,7 +106,7 @@ public async Task ClearCookiesAsync(APIRequestContextClearCookiesOptions? option options?.PathRegex); await SendMessageToServerAsync("clearCookies", @params).ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task FetchAsync(IRequest request, APIRequestContextOptions? options = null) @@ -132,7 +132,7 @@ internal async Task InnerFetchAsync(IRequest request, string? urlO } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task FetchAsync(string url, APIRequestContextOptions? options = null) + public Task FetchAsync(string url, APIRequestContextOptions? options = null) => WrapApiCallAsync(async () => { if (!_closeReason.IsNullOrEmpty()) { @@ -194,7 +194,7 @@ public async Task FetchAsync(string url, APIRequestContextOptions? var response = await SendMessageToServerAsync("fetch", message, timeout: _timeoutSettings.Timeout(options?.Timeout)).ConfigureAwait(false); return new APIResponse(this, response?.GetProperty("response").ToObject()!); - } + }); private bool IsJsonContentType(IDictionary? headers) { @@ -249,7 +249,7 @@ private APIRequestContextOptions WithMethod(APIRequestContextOptions? options, s } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task StorageStateAsync(APIRequestContextStorageStateOptions? options = null) + public Task StorageStateAsync(APIRequestContextStorageStateOptions? options = null) => WrapApiCallAsync(async () => { string state = JsonSerializer.Serialize( await SendMessageToServerAsync( @@ -267,7 +267,7 @@ await SendMessageToServerAsync( } return state; - } + }); IFormData IAPIRequestContext.CreateFormData() => new FormData(); } diff --git a/src/Playwright/Core/Artifact.cs b/src/Playwright/Core/Artifact.cs index ac3951802f..04ba134a31 100644 --- a/src/Playwright/Core/Artifact.cs +++ b/src/Playwright/Core/Artifact.cs @@ -43,7 +43,7 @@ internal Artifact(ChannelOwner parent, string guid, ArtifactInitializer initiali internal string AbsolutePath { get; } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task PathAfterFinishedAsync() + public Task PathAfterFinishedAsync() => WrapApiCallAsync(async () => { if (_connection.IsRemote) { @@ -51,10 +51,10 @@ public async Task PathAfterFinishedAsync() } return (await SendMessageToServerAsync("pathAfterFinished") .ConfigureAwait(false)).GetProperty("value").ToString(); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task SaveAsAsync(string path) + public Task SaveAsAsync(string path) => WrapApiCallAsync(async () => { if (!_connection.IsRemote) { @@ -76,15 +76,15 @@ await SendMessageToServerAsync( await stream.StreamImpl.CopyToAsync(fileStream, bufferSize: 1024 * 1024).ConfigureAwait(false); } } - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task CreateReadStreamAsync() + public Task CreateReadStreamAsync() => WrapApiCallAsync(async () => { var stream = (await SendMessageToServerAsync("stream") .ConfigureAwait(false)).GetObject("stream", _connection); return stream.StreamImpl; - } + }); internal Task CancelAsync() => SendMessageToServerAsync("cancel"); diff --git a/src/Playwright/Core/Browser.cs b/src/Playwright/Core/Browser.cs index e5e2b874dd..fb927dba25 100644 --- a/src/Playwright/Core/Browser.cs +++ b/src/Playwright/Core/Browser.cs @@ -39,7 +39,7 @@ internal class Browser : ChannelOwner, IBrowser { private readonly BrowserInitializer _initializer; private readonly TaskCompletionSource _closedTcs = new(); - internal readonly List _contexts = new(); + internal List _contexts = new(); internal string? _tracesDir = null; internal BrowserType _browserType = null!; internal string? _closeReason; @@ -81,7 +81,7 @@ internal override void OnMessage(string method, JsonElement serverParams) } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task CloseAsync(BrowserCloseOptions? options = default) + public Task CloseAsync(BrowserCloseOptions? options = default) => WrapApiCallAsync(async () => { _closeReason = options?.Reason; try @@ -103,10 +103,10 @@ public async Task CloseAsync(BrowserCloseOptions? options = default) { // Swallow exception } - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task NewContextAsync(BrowserNewContextOptions? options = default) + public Task NewContextAsync(BrowserNewContextOptions? options = default) => WrapApiCallAsync(async () => { options ??= new(); @@ -174,10 +174,10 @@ public async Task NewContextAsync(BrowserNewContextOptions? opt var context = await SendMessageToServerAsync("newContext", args).ConfigureAwait(false); await context.InitializeHarFromOptionsAsync(options).ConfigureAwait(false); return context; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task NewPageAsync(BrowserNewPageOptions? options = default) + public Task NewPageAsync(BrowserNewPageOptions? options = default) => WrapApiCallAsync(async () => { options ??= new(); @@ -235,7 +235,7 @@ public async Task NewPageAsync(BrowserNewPageOptions? options = default) }, false, "Create page").ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public ValueTask DisposeAsync() => new ValueTask(CloseAsync()); @@ -277,6 +277,7 @@ public async Task NewPageAsync(BrowserNewPageOptions? options = default) internal void ConnectToBrowserType(BrowserType browserType, string? tracesDir) { + _connection.Dispatcher.AssertCurrent(); // Note: when using connect(), `browserType` is different from `this.parent`. // This is why browser type is not wired up in the constructor, and instead this separate method is called later on. _browserType = browserType; @@ -285,21 +286,22 @@ internal void ConnectToBrowserType(BrowserType browserType, string? tracesDir) { context._tracing._tracesDir = this._tracesDir; context._request._tracing._tracesDir = this._tracesDir; - browserType.Playwright._selectors._contextsForSelectors.Add(context); + CopyOnWrite.Add(ref browserType.Playwright._selectors._contextsForSelectors, context); } } private void DidCreateContext(BrowserContext context) { + _connection.Dispatcher.AssertCurrent(); context._browser = this; - _contexts.Add(context); + CopyOnWrite.Add(ref _contexts, context); // Note: when connecting to a browser, initial contexts arrive before `_browserType` is set, // and will be configured later in `ConnectToBrowserType`. if (_browserType != null) { context._tracing._tracesDir = _tracesDir; context._request._tracing._tracesDir = _tracesDir; - _browserType.Playwright._selectors._contextsForSelectors.Add(context); + CopyOnWrite.Add(ref _browserType.Playwright._selectors._contextsForSelectors, context); } Context?.Invoke(this, context); } @@ -313,11 +315,11 @@ internal void DidClose() _connection.Close -= _onConnectionClose; IsConnected = false; Disconnected?.Invoke(this, this); - _closedTcs.TrySetResult(true); + _connection.Dispatcher.Complete(_closedTcs, true); } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task BindAsync(string title, BrowserBindOptions? options = default) + public Task BindAsync(string title, BrowserBindOptions? options = default) => WrapApiCallAsync(async () => { options ??= new(); var result = await SendMessageToServerAsync("startServer", new Dictionary @@ -328,7 +330,7 @@ public async Task BindAsync(string title, BrowserBindOptions? ["port"] = options.Port, }).ConfigureAwait(false); return new BrowserBindResult { Endpoint = result!.Value.GetProperty("endpoint").GetString()! }; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task UnbindAsync() diff --git a/src/Playwright/Core/BrowserContext.cs b/src/Playwright/Core/BrowserContext.cs index ce612cb2b7..ec64152dc0 100644 --- a/src/Playwright/Core/BrowserContext.cs +++ b/src/Playwright/Core/BrowserContext.cs @@ -48,10 +48,10 @@ internal class BrowserContext : ChannelOwner, IBrowserContext internal readonly Tracing _tracing; private readonly Clock _clock; internal readonly APIRequestContext _request; - internal readonly List _serviceWorkers = new(); + internal List _serviceWorkers = new(); private readonly List _webSocketRoutes = new(); private List _routes = new(); - internal readonly List _pages = new(); + internal List _pages = new(); // Browser is null for browser contexts created outside of normal browser, e.g. android or electron. internal Browser? _browser; private readonly List _harRouters = new(); @@ -91,20 +91,20 @@ internal BrowserContext(ChannelOwner parent, string guid, BrowserContextInitiali public event EventHandler? Console { - add => this._consoleImpl = UpdateEventHandler("console", this._consoleImpl, value, true); - remove => this._consoleImpl = UpdateEventHandler("console", this._consoleImpl, value, false); + add => UpdateEventHandler("console", ref _consoleImpl, value, true, () => _consoleImpl != null); + remove => UpdateEventHandler("console", ref _consoleImpl, value, false, () => _consoleImpl != null); } public event EventHandler Dialog { - add => this._dialogImpl = UpdateEventHandler("dialog", this._dialogImpl, value, true); - remove => this._dialogImpl = UpdateEventHandler("dialog", this._dialogImpl, value, false); + add => UpdateEventHandler("dialog", ref _dialogImpl, value, true, () => _dialogImpl != null); + remove => UpdateEventHandler("dialog", ref _dialogImpl, value, false, () => _dialogImpl != null); } public event EventHandler DialogClosed { - add => this._dialogClosedImpl = UpdateEventHandler("dialogClosed", this._dialogClosedImpl, value, true); - remove => this._dialogClosedImpl = UpdateEventHandler("dialogClosed", this._dialogClosedImpl, value, false); + add => UpdateEventHandler("dialogClosed", ref _dialogClosedImpl, value, true, () => _dialogClosedImpl != null); + remove => UpdateEventHandler("dialogClosed", ref _dialogClosedImpl, value, false, () => _dialogClosedImpl != null); } public event EventHandler? Page; @@ -127,26 +127,26 @@ public event EventHandler DialogClosed public event EventHandler? Request { - add => this._requestImpl = UpdateEventHandler("request", this._requestImpl, value, true); - remove => this._requestImpl = UpdateEventHandler("request", this._requestImpl, value, false); + add => UpdateEventHandler("request", ref _requestImpl, value, true, () => _requestImpl != null); + remove => UpdateEventHandler("request", ref _requestImpl, value, false, () => _requestImpl != null); } public event EventHandler Response { - add => this._responseImpl = UpdateEventHandler("response", this._responseImpl, value, true); - remove => this._responseImpl = UpdateEventHandler("response", this._responseImpl, value, false); + add => UpdateEventHandler("response", ref _responseImpl, value, true, () => _responseImpl != null); + remove => UpdateEventHandler("response", ref _responseImpl, value, false, () => _responseImpl != null); } public event EventHandler RequestFinished { - add => this._requestFinishedImpl = UpdateEventHandler("requestFinished", this._requestFinishedImpl, value, true); - remove => this._requestFinishedImpl = UpdateEventHandler("requestFinished", this._requestFinishedImpl, value, false); + add => UpdateEventHandler("requestFinished", ref _requestFinishedImpl, value, true, () => _requestFinishedImpl != null); + remove => UpdateEventHandler("requestFinished", ref _requestFinishedImpl, value, false, () => _requestFinishedImpl != null); } public event EventHandler RequestFailed { - add => this._requestFailedImpl = UpdateEventHandler("requestFailed", this._requestFailedImpl, value, true); - remove => this._requestFailedImpl = UpdateEventHandler("requestFailed", this._requestFailedImpl, value, false); + add => UpdateEventHandler("requestFailed", ref _requestFailedImpl, value, true, () => _requestFailedImpl != null); + remove => UpdateEventHandler("requestFailed", ref _requestFailedImpl, value, false, () => _requestFailedImpl != null); } public event EventHandler? ServiceWorker; @@ -245,7 +245,7 @@ internal override void OnMessage(string method, JsonElement serverParams) { var serviceWorker = serverParams.GetProperty("worker").ToObject(_connection.DefaultJsonSerializerOptions)!; ((Worker)serviceWorker).Context = this; - _serviceWorkers.Add(serviceWorker); + CopyOnWrite.Add(ref _serviceWorkers, serviceWorker); ServiceWorker?.Invoke(this, serviceWorker); break; } @@ -346,7 +346,7 @@ public Task AddCookiesAsync(IEnumerable cookies) => SendMessageToServerA }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task AddInitScriptAsync(string? script = null, string? scriptPath = null) + public Task AddInitScriptAsync(string? script = null, string? scriptPath = null) => WrapApiCallAsync(async () => { if (string.IsNullOrEmpty(script)) { @@ -360,10 +360,10 @@ public async Task AddInitScriptAsync(string? script = null, st ["source"] = script, }).ConfigureAwait(false); return result.GetObject("disposable", _connection)!; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task ClearCookiesAsync(BrowserContextClearCookiesOptions? options = default) + public Task ClearCookiesAsync(BrowserContextClearCookiesOptions? options = default) => WrapApiCallAsync(async () => { var @params = ToClearCookiesParams( options?.Name ?? options?.NameString, @@ -374,7 +374,7 @@ public async Task ClearCookiesAsync(BrowserContextClearCookiesOptions? options = options?.PathRegex); await SendMessageToServerAsync("clearCookies", @params).ConfigureAwait(false); - } + }); internal static Dictionary ToClearCookiesParams(string? name, Regex? nameRegex, string? domain, Regex? domainRegex, string? path, Regex? pathRegex) { @@ -399,7 +399,7 @@ public async Task ClearCookiesAsync(BrowserContextClearCookiesOptions? options = public Task ClearPermissionsAsync() => SendMessageToServerAsync("clearPermissions"); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task CloseAsync(BrowserContextCloseOptions? options = default) + public Task CloseAsync(BrowserContextCloseOptions? options = default) => WrapApiCallAsync(async () => { if (ClosingOrClosed) { @@ -428,7 +428,7 @@ await WrapApiCallAsync( { ExceptionDispatchInfo.Capture(harError).Throw(); } - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task> CookiesAsync() => CookiesAsync(Array.Empty()); @@ -531,7 +531,7 @@ public async Task NewCDPSessionAsync(IFrame frame) }).ConfigureAwait(false); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task NewPageAsync() + public Task NewPageAsync() => WrapApiCallAsync(async () => { if (OwnerPage != null) { @@ -539,7 +539,7 @@ public async Task NewPageAsync() } return await SendMessageToServerAsync("newPage").ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task RouteAsync(string globMatch, Func handler, BrowserContextRouteOptions? options = null) @@ -591,7 +591,7 @@ public Task SetOfflineAsync(bool offline) => SendMessageToServerAsync( }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task StorageStateAsync(BrowserContextStorageStateOptions? options = default) + public Task StorageStateAsync(BrowserContextStorageStateOptions? options = default) => WrapApiCallAsync(async () => { string state = JsonSerializer.Serialize( await SendMessageToServerAsync( @@ -610,10 +610,10 @@ await SendMessageToServerAsync( } return state; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task SetStorageStateAsync(string storageStatePath) + public Task SetStorageStateAsync(string storageStatePath) => WrapApiCallAsync(async () => { if (!File.Exists(storageStatePath)) { @@ -627,14 +627,14 @@ await SendMessageToServerAsync( { ["storageState"] = storageState, }).ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task UnrouteAllAsync(BrowserContextUnrouteAllOptions? options = default) + public Task UnrouteAllAsync(BrowserContextUnrouteAllOptions? options = default) => WrapApiCallAsync(async () => { await UnrouteInternalAsync(_routes, new(), options?.Behavior).ConfigureAwait(false); DisposeHarRouters(); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task UnrouteAsync(string? globMatch, Action? handler) @@ -665,7 +665,7 @@ public Task UnrouteAsync(Func funcMatch, Func handle return _closeReason ?? _browser?._closeReason; } - internal async Task InnerWaitForEventAsync(PlaywrightEvent playwrightEvent, Func? action = default, Func? predicate = default, float? timeout = default) + internal Task InnerWaitForEventAsync(PlaywrightEvent playwrightEvent, Func? action = default, Func? predicate = default, float? timeout = default) => WrapApiCallAsync(async () => { if (playwrightEvent == null) { @@ -688,7 +688,7 @@ internal async Task InnerWaitForEventAsync(PlaywrightEvent playwrightEv } return await result.ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task WaitForPageAsync(BrowserContextWaitForPageOptions? options = default) @@ -750,6 +750,7 @@ internal async Task OnRouteAsync(Route route) _routes.Remove(routeHandler); } var handled = await routeHandler.HandleAsync(route).ConfigureAwait(false); + _connection.Dispatcher.AssertCurrent(); if (_routes.Count == 0) { UpdateInterceptionAsync().IgnoreException(); @@ -791,7 +792,7 @@ internal bool UrlMatches(string url, string globMatch) baseURL = BaseURL, }.Match(url); - private async Task RouteAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate handler, BrowserContextRouteOptions? options) + private Task RouteAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate handler, BrowserContextRouteOptions? options) => WrapApiCallAsync(async () => { var setting = new RouteHandler() { @@ -805,15 +806,16 @@ private async Task RouteAsync(string? globMatch, Regex? reMatc Handler = handler, Times = options?.Times, }; + _connection.Dispatcher.AssertCurrent(); _routes.Insert(0, setting); await UpdateInterceptionAsync().ConfigureAwait(false); return new DisposableStub(async () => { await UnrouteAsync(globMatch, reMatch, funcMatch, handler).ConfigureAwait(false); }); - } + }); - private async Task UnrouteAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate? handler) + private Task UnrouteAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate? handler) => WrapApiCallAsync(async () => { var removed = new List(); var remaining = new List(); @@ -829,10 +831,11 @@ private async Task UnrouteAsync(string? globMatch, Regex? reMatch, Func removed, List remaining, UnrouteBehavior? behavior) { + _connection.Dispatcher.AssertCurrent(); _routes = remaining; if (behavior != null && behavior != UnrouteBehavior.Default) { @@ -855,18 +858,19 @@ await SendMessageToServerAsync( internal void OnClose() { + _connection.Dispatcher.AssertCurrent(); ClosingOrClosed = true; if (Browser != null) { - ((Browser)Browser)._contexts.Remove(this); - ((Browser)Browser)._browserType.Playwright._selectors._contextsForSelectors.Remove(this); + CopyOnWrite.Remove(ref ((Browser)Browser)._contexts, this); + CopyOnWrite.Remove(ref ((Browser)Browser)._browserType.Playwright._selectors._contextsForSelectors, this); } DisposeHarRouters(); _tracing.ResetStackCounter(); _request._tracing.ResetStackCounter(); Close?.Invoke(this, this); - _closeTcs.TrySetResult(true); + _connection.Dispatcher.Complete(_closeTcs, true); } internal void FirePageClose(IPage page) => PageClose?.Invoke(this, page); @@ -883,8 +887,9 @@ internal void OnClose() private void Channel_OnPage(object sender, Page page) { + _connection.Dispatcher.AssertCurrent(); page.Context = this; - _pages.Add(page); + CopyOnWrite.Add(ref _pages, page); Page?.Invoke(this, page); if (page.Opener?.IsClosed == false) @@ -903,7 +908,7 @@ private void Channel_BindingCall(BindingCall bindingCall) private void Channel_Route(object sender, Route route) => _ = OnRouteAsync(route).ConfigureAwait(false); - private async Task ExposeBindingAsync(string name, Delegate callback) + private Task ExposeBindingAsync(string name, Delegate callback) => WrapApiCallAsync(async () => { foreach (var page in _pages) { @@ -927,7 +932,7 @@ private async Task ExposeBindingAsync(string name, Delegate ca ["name"] = name, }).ConfigureAwait(false); return result.GetObject("disposable", _connection)!; - } + }); private HarContentPolicy? RouteFromHarUpdateContentPolicyToHarContentPolicy(RouteFromHarUpdateContentPolicy? policy) { @@ -946,7 +951,7 @@ internal Task RecordIntoHarAsync(string har, Page? page, BrowserContextR => _tracing.RecordIntoHarAsync(har, page, contentPolicy ?? RouteFromHarUpdateContentPolicyToHarContentPolicy(options?.UpdateContent), options?.Url ?? options?.UrlString, options?.UrlRegex, options?.UpdateMode ?? HarMode.Minimal, resourcesDir); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task RouteFromHARAsync(string har, BrowserContextRouteFromHAROptions? options = null) + public Task RouteFromHARAsync(string har, BrowserContextRouteFromHAROptions? options = null) => WrapApiCallAsync(async () => { if (options?.Update == true) { @@ -959,9 +964,10 @@ public async Task RouteFromHARAsync(string har, BrowserContextRouteFromHAROption UrlRegex = options?.UrlRegex, UrlString = options?.UrlString, }).ConfigureAwait(false); + _connection.Dispatcher.AssertCurrent(); _harRouters.Add(harRouter); await harRouter.AddContextRouteAsync(this).ConfigureAwait(false); - } + }); private void DisposeHarRouters() { @@ -984,8 +990,9 @@ public Task RouteWebSocketAsync(Regex url, Action handler) public Task RouteWebSocketAsync(Func url, Action handler) => RouteWebSocketAsync(null, null, url, handler); - private Task RouteWebSocketAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate handler) + private Task RouteWebSocketAsync(string? globMatch, Regex? reMatch, Func? funcMatch, Delegate handler) => WrapApiCallAsync(() => { + _connection.Dispatcher.AssertCurrent(); _webSocketRoutes.Insert(0, new WebSocketRouteHandler() { urlMatcher = new URLMatch() @@ -999,7 +1006,7 @@ private Task RouteWebSocketAsync(string? globMatch, Regex? reMatch, Func _initializer.Name; [MethodImpl(MethodImplOptions.NoInlining)] - public async Task LaunchAsync(BrowserTypeLaunchOptions? options = default) + public Task LaunchAsync(BrowserTypeLaunchOptions? options = default) => WrapApiCallAsync(async () => { options ??= new BrowserTypeLaunchOptions(); Browser browser = await SendMessageToServerAsync( @@ -78,10 +78,10 @@ public async Task LaunchAsync(BrowserTypeLaunchOptions? options = defa timeout: TimeoutSettings.LaunchTimeout(options.Timeout)).ConfigureAwait(false); browser.ConnectToBrowserType(this, options.TracesDir); return browser; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task LaunchPersistentContextAsync(string userDataDir, BrowserTypeLaunchPersistentContextOptions? options = default) + public Task LaunchPersistentContextAsync(string userDataDir, BrowserTypeLaunchPersistentContextOptions? options = default) => WrapApiCallAsync(async () => { options ??= new BrowserTypeLaunchPersistentContextOptions(); var channelArgs = new Dictionary @@ -161,10 +161,10 @@ await context.InitializeHarFromOptionsAsync(new() RecordHarUrlFilterString = options.RecordHarUrlFilterString, }).ConfigureAwait(false); return context; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task ConnectAsync(string wsEndpoint, BrowserTypeConnectOptions? options = null) + public Task ConnectAsync(string wsEndpoint, BrowserTypeConnectOptions? options = null) => WrapApiCallAsync(async () => { options ??= new BrowserTypeConnectOptions(); var headers = new List>(options.Headers ?? new Dictionary()) @@ -178,11 +178,10 @@ void ClosePipe() { pipe.CloseAsync().IgnoreException(); } - var connection = new Connection(_connection.LocalUtils); + var connection = new Connection(_connection.LocalUtils, _connection.Dispatcher); connection.MarkAsRemote(); connection.Close += (_, _) => ClosePipe(); - string? closeError = null; Browser? browser = null; void OnPipeClosed(string? reason = null) { @@ -196,7 +195,7 @@ void OnPipeClosed(string? reason = null) context.OnClose(); } browser?.DidClose(); - connection.DoClose(reason ?? closeError); + connection.DoClose(reason); // TODO: Backport https://github.com/microsoft/playwright/commit/d8d5289e8692c9b1265d23ee66988d1ac5122f33 // Give a chance to any API call promises to reject upon page/context closure. // This happens naturally when we receive page.onClose and browser.onClose from the server @@ -218,22 +217,13 @@ void OnPipeClosed(string? reason = null) } catch { + // The send failed on a thread pool thread; closing touches page and context state. + await connection.Dispatcher.SwitchTo(); OnPipeClosed(); } }; - pipe.Message += (_, message) => - { - try - { - connection.Dispatch(message); - } - catch (Exception ex) - { - closeError = ex.ToString(); - ClosePipe(); - } - }; + pipe.Message += (_, message) => connection.Dispatch(message); async Task CreateBrowserAsync() { @@ -252,10 +242,10 @@ async Task CreateBrowserAsync() } var task = CreateBrowserAsync(); return await task.WithTimeout(timeout, _ => throw new TimeoutException($"BrowserType.ConnectAsync: Timeout {timeout}ms exceeded")).ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task ConnectOverCDPAsync(string endpointURL, BrowserTypeConnectOverCDPOptions? options = null) + public Task ConnectOverCDPAsync(string endpointURL, BrowserTypeConnectOverCDPOptions? options = null) => WrapApiCallAsync(async () => { if (Name != "chromium") { @@ -277,5 +267,5 @@ public async Task ConnectOverCDPAsync(string endpointURL, BrowserTypeC Browser browser = result.GetProperty("browser").ToObject(_connection.DefaultJsonSerializerOptions); browser.ConnectToBrowserType(this, null); return browser; - } + }); } diff --git a/src/Playwright/Core/CDPSession.cs b/src/Playwright/Core/CDPSession.cs index 42f5ae6e90..f975c24c98 100644 --- a/src/Playwright/Core/CDPSession.cs +++ b/src/Playwright/Core/CDPSession.cs @@ -60,7 +60,7 @@ internal override void OnMessage(string method, JsonElement serverParams) public Task DetachAsync() => SendMessageToServerAsync("detach"); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task SendAsync(string method, Dictionary? args = null, CDPSessionSendOptions? options = null) + public Task SendAsync(string method, Dictionary? args = null, CDPSessionSendOptions? options = null) => WrapApiCallAsync(async () => { var newArgs = new Dictionary() { { "method", method } }; if (args != null) @@ -69,7 +69,7 @@ internal override void OnMessage(string method, JsonElement serverParams) } var result = await SendMessageToServerAsync("send", newArgs, timeout: options?.Timeout ?? 0).ConfigureAwait(false); return result?.GetProperty("result"); - } + }); private void OnCDPEvent(string name, JsonElement? @params) { diff --git a/src/Playwright/Core/Dialog.cs b/src/Playwright/Core/Dialog.cs index a343abb24e..617d271073 100644 --- a/src/Playwright/Core/Dialog.cs +++ b/src/Playwright/Core/Dialog.cs @@ -58,7 +58,7 @@ public Task AcceptAsync(string? promptText = null) => SendMessageToServerAsync( }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task DismissAsync() + public Task DismissAsync() => WrapApiCallAsync(async () => { try { @@ -67,5 +67,5 @@ public async Task DismissAsync() catch (Exception e) when (DriverMessages.IsTargetClosedError(e)) { } - } + }); } diff --git a/src/Playwright/Core/ElementHandle.cs b/src/Playwright/Core/ElementHandle.cs index 1fed26521b..9044cc1c46 100644 --- a/src/Playwright/Core/ElementHandle.cs +++ b/src/Playwright/Core/ElementHandle.cs @@ -96,7 +96,7 @@ public Task TypeAsync(string text, ElementHandleTypeOptions? options = default) }, timeout: _frame.Timeout(options?.Timeout)); - public async Task ScreenshotAsync(ElementHandleScreenshotOptions? options = default) + public Task ScreenshotAsync(ElementHandleScreenshotOptions? options = default) => WrapApiCallAsync(async () => { options ??= new(); if (options.Type == null && !string.IsNullOrEmpty(options.Path) && options.Path != null) @@ -134,7 +134,7 @@ public async Task ScreenshotAsync(ElementHandleScreenshotOptions? option } return result; - } + }); public Task FillAsync(string value, ElementHandleFillOptions? options = default) => SendMessageToServerAsync( @@ -171,7 +171,7 @@ public Task ScrollIntoViewIfNeededAsync(ElementHandleScrollIntoViewIfNeededOptio public async Task OwnerFrameAsync() => await SendMessageToServerAsync("ownerFrame").ConfigureAwait(false); - public async Task BoundingBoxAsync() + public Task BoundingBoxAsync() => WrapApiCallAsync(async () => { var result = (await SendMessageToServerAsync("boundingBox").ConfigureAwait(false))!.Value; if (result.TryGetProperty("value", out var value)) @@ -179,7 +179,7 @@ public Task ScrollIntoViewIfNeededAsync(ElementHandleScrollIntoViewIfNeededOptio return value.ToObject(); } return null; - } + }); public Task ClickAsync(ElementHandleClickOptions? options = default) => SendMessageToServerAsync( @@ -220,7 +220,7 @@ public Task DblClickAsync(ElementHandleDblClickOptions? options = default) public Task SetInputFilesAsync(string files, ElementHandleSetInputFilesOptions? options = default) => SetInputFilesAsync(new[] { files }, options); - public async Task SetInputFilesAsync(IEnumerable files, ElementHandleSetInputFilesOptions? options = default) + public Task SetInputFilesAsync(IEnumerable files, ElementHandleSetInputFilesOptions? options = default) => WrapApiCallAsync(async () => { var frame = await OwnerFrameAsync().ConfigureAwait(false); if (frame == null) @@ -239,12 +239,12 @@ await SendMessageToServerAsync( ["directoryStream"] = converted.DirectoryStream, }, timeout: _frame.Timeout(options?.Timeout)).ConfigureAwait(false); - } + }); public Task SetInputFilesAsync(FilePayload files, ElementHandleSetInputFilesOptions? options = default) => SetInputFilesAsync(new[] { files }, options); - public async Task SetInputFilesAsync(IEnumerable files, ElementHandleSetInputFilesOptions? options = default) + public Task SetInputFilesAsync(IEnumerable files, ElementHandleSetInputFilesOptions? options = default) => WrapApiCallAsync(async () => { var converted = SetInputFilesHelpers.ConvertInputFiles(files); await SendMessageToServerAsync( @@ -256,7 +256,7 @@ await SendMessageToServerAsync( ["streams"] = converted.Streams, }, timeout: _frame.Timeout(options?.Timeout)).ConfigureAwait(false); - } + }); public async Task QuerySelectorAsync(string selector) => await SendMessageToServerAsync( @@ -313,7 +313,7 @@ public Task DispatchEventAsync(string type, object? eventInit = null) ["eventInit"] = ScriptsHelper.SerializedArgument(eventInit), }); - public async Task GetAttributeAsync(string name) + public Task GetAttributeAsync(string name) => WrapApiCallAsync(async () => { if ((await SendMessageToServerAsync("getAttribute", new Dictionary { @@ -323,7 +323,7 @@ public Task DispatchEventAsync(string type, object? eventInit = null) return value.ToString(); } return null; - } + }); public async Task InnerHTMLAsync() => (await SendMessageToServerAsync("innerHTML").ConfigureAwait(false))!.Value.GetProperty("value").ToString(); diff --git a/src/Playwright/Core/Frame.cs b/src/Playwright/Core/Frame.cs index bf7a34f202..b5ca6e01e2 100644 --- a/src/Playwright/Core/Frame.cs +++ b/src/Playwright/Core/Frame.cs @@ -43,7 +43,7 @@ namespace Microsoft.Playwright.Core; internal class Frame : ChannelOwner, IFrame { private readonly List _loadStates = new(); - internal readonly List _childFrames = new(); + internal List _childFrames = new(); internal Frame(ChannelOwner parent, string guid, FrameInitializer initializer) : base(parent, guid) { @@ -112,6 +112,7 @@ internal override void OnMessage(string method, JsonElement serverParams) internal void OnLoadState(WaitUntilState? add, WaitUntilState? remove) { + _connection.Dispatcher.AssertCurrent(); if (add.HasValue) { _loadStates.Add(add.Value); @@ -225,7 +226,7 @@ internal async Task> SelectOptionAsync(string selector, IE timeout: Timeout(options?.Timeout)).ConfigureAwait(false))!.Value.GetProperty("values").ToObject().ToList().AsReadOnly(); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task WaitForLoadStateAsync(LoadState? state = default, FrameWaitForLoadStateOptions? options = default) + public Task WaitForLoadStateAsync(LoadState? state = default, FrameWaitForLoadStateOptions? options = default) => WrapApiCallAsync(async () => { Waiter? waiter = null; WaitUntilState loadState = Microsoft.Playwright.WaitUntilState.Load; @@ -245,6 +246,7 @@ public async Task WaitForLoadStateAsync(LoadState? state = default, FrameWaitFor { waiter = SetupNavigationWaiter("frame.WaitForLoadStateAsync", options?.Timeout); + _connection.Dispatcher.AssertCurrent(); if (_loadStates.Contains(loadState)) { waiter.Log($" not waiting, \"{state}\" event already fired"); @@ -262,7 +264,7 @@ await waiter.WaitForEventAsync(this, "LoadState", s => { waiter?.Dispose(); } - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task WaitForNavigationAsync(FrameWaitForNavigationOptions? options = default) @@ -279,7 +281,7 @@ await waiter.WaitForEventAsync(this, "LoadState", s => } [MethodImpl(MethodImplOptions.NoInlining)] - public async Task RunAndWaitForNavigationAsync(Func? action, FrameRunAndWaitForNavigationOptions? options = default) + public Task RunAndWaitForNavigationAsync(Func? action, FrameRunAndWaitForNavigationOptions? options = default) => WrapApiCallAsync(async () => { using var waiter = SetupNavigationWaiter("frame.WaitForNavigationAsync", options?.Timeout); var result = WaitForNavigationInternalAsync(waiter, options?.Url ?? options?.UrlString, options?.UrlFunc, options?.UrlRegex, options?.WaitUntil); @@ -290,7 +292,7 @@ await waiter.WaitForEventAsync(this, "LoadState", s => } return await result.ConfigureAwait(false); - } + }); private async Task WaitForNavigationInternalAsync( Waiter waiter, @@ -333,6 +335,7 @@ await waiter.WaitForEventAsync(this, "LoadState", s => await waiter.WaitForPromiseAsync(Task.FromException(ex)).ConfigureAwait(false); } + _connection.Dispatcher.AssertCurrent(); if (!_loadStates.Select(s => s.ToValueString()).Contains(waitUntil.Value.ToValueString())) { await waiter.WaitForEventAsync( @@ -411,7 +414,7 @@ public Task TypeAsync(string selector, string text, FrameTypeOptions? options = timeout: Timeout(options?.Timeout)); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task GetAttributeAsync(string selector, string name, FrameGetAttributeOptions? options = default) + public Task GetAttributeAsync(string selector, string name, FrameGetAttributeOptions? options = default) => WrapApiCallAsync(async () => { if ((await SendMessageToServerAsync( "getAttribute", @@ -426,7 +429,7 @@ public Task TypeAsync(string selector, string text, FrameTypeOptions? options = return retValue.ToString(); } return null; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public async Task InnerHTMLAsync(string selector, FrameInnerHTMLOptions? options = default) @@ -520,7 +523,7 @@ public Task FillAsync(string selector, string value, FrameFillOptions? options = timeout: Timeout(options?.Timeout)); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task AddScriptTagAsync(FrameAddScriptTagOptions? options = default) + public Task AddScriptTagAsync(FrameAddScriptTagOptions? options = default) => WrapApiCallAsync(async () => { var content = options?.Content; if (!string.IsNullOrEmpty(options?.Path) && options?.Path != null) @@ -535,10 +538,10 @@ public async Task AddScriptTagAsync(FrameAddScriptTagOptions? op ["content"] = content, ["type"] = options?.Type, }).ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task AddStyleTagAsync(FrameAddStyleTagOptions? options = default) + public Task AddStyleTagAsync(FrameAddStyleTagOptions? options = default) => WrapApiCallAsync(async () => { var content = options?.Content; if (!string.IsNullOrEmpty(options?.Path) && options?.Path != null) @@ -553,33 +556,33 @@ public async Task AddStyleTagAsync(FrameAddStyleTagOptions? opti ["path"] = options?.Path, ["content"] = content, }).ConfigureAwait(false); - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task SetInputFilesAsync(string selector, string files, FrameSetInputFilesOptions? options = default) => SetInputFilesAsync(selector, new[] { files }, options); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task SetInputFilesAsync(string selector, IEnumerable files, FrameSetInputFilesOptions? options = default) + public Task SetInputFilesAsync(string selector, IEnumerable files, FrameSetInputFilesOptions? options = default) => WrapApiCallAsync(async () => { var converted = await SetInputFilesHelpers.ConvertInputFilesAsync(files, (BrowserContext)Page.Context).ConfigureAwait(false); #pragma warning disable CS0612 // Type or member is obsolete await _setInputFilesAsync(selector, converted, options?.NoWaitAfter, options?.Timeout, options?.Strict).ConfigureAwait(false); #pragma warning restore CS0612 // Type or member is obsolete - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public Task SetInputFilesAsync(string selector, FilePayload files, FrameSetInputFilesOptions? options = default) => SetInputFilesAsync(selector, new[] { files }, options); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task SetInputFilesAsync(string selector, IEnumerable files, FrameSetInputFilesOptions? options = default) + public Task SetInputFilesAsync(string selector, IEnumerable files, FrameSetInputFilesOptions? options = default) => WrapApiCallAsync(async () => { var converted = SetInputFilesHelpers.ConvertInputFiles(files); #pragma warning disable CS0612 // Type or member is obsolete await _setInputFilesAsync(selector, converted, noWaitAfter: options?.NoWaitAfter, timeout: options?.Timeout, options?.Strict).ConfigureAwait(false); #pragma warning restore CS0612 // Type or member is obsolete - } + }); private async Task _setInputFilesAsync(string selector, SetInputFilesFiles files, bool? noWaitAfter, float? timeout, bool? strict) { @@ -1058,7 +1061,7 @@ await SendMessageToServerAsync( } } - private Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func? urlFunc, FrameWaitForURLOptions? options = default) + private Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func? urlFunc, FrameWaitForURLOptions? options = default) => WrapApiCallAsync(() => { if (new URLMatch() { @@ -1085,7 +1088,7 @@ private Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func GetPropertyAsync(string propertyName) => await Send }).ConfigureAwait(false); [MethodImpl(MethodImplOptions.NoInlining)] - public async Task> GetPropertiesAsync() + public Task> GetPropertiesAsync() => WrapApiCallAsync>(async () => { var result = new Dictionary(); var channelResult = (await SendMessageToServerAsync("getPropertyList").ConfigureAwait(false))!.Value @@ -99,7 +99,7 @@ public async Task> GetPropertiesAsync() } return result; - } + }); [MethodImpl(MethodImplOptions.NoInlining)] public async ValueTask DisposeAsync() diff --git a/src/Playwright/Core/Locator.cs b/src/Playwright/Core/Locator.cs index d7a2bbacd4..ca64882d33 100644 --- a/src/Playwright/Core/Locator.cs +++ b/src/Playwright/Core/Locator.cs @@ -205,11 +205,11 @@ public Task FillAsync(string value, LocatorFillOptions? options = null) public Task ClearAsync(LocatorClearOptions? options = null) => this._frame.WrapApiCallAsync(() => _frame.FillAsync(_selector, string.Empty, ConvertOptions(options)), false, "Clear"); - public async Task HighlightAsync(LocatorHighlightOptions? options = default) + public Task HighlightAsync(LocatorHighlightOptions? options = default) => _frame.WrapApiCallAsync(async () => { await _frame.HighlightAsync(_selector, options?.Style).ConfigureAwait(false); return new DisposableStub(() => HideHighlightAsync()); - } + }); public Task HideHighlightAsync() => _frame.HideHighlightAsync(_selector); @@ -431,6 +431,12 @@ public override string ToString() { if (_stringRepresentation == null) { + if (_frame._connection.Dispatcher.IsCurrent) + { + // Called from an event handler: blocking here would wait for a reply that only + // this thread can deliver. + return "Locator@" + _selector; + } try { // ToString() must be synchronous, so block on the driver call. The result is cached. @@ -701,7 +707,7 @@ private static string EscapeForTextSelector(string text, bool? exact) async Task> ILocator.AllAsync() => Enumerable.Range(0, await CountAsync().ConfigureAwait(false)).Select(Nth).ToArray(); - public async Task AriaSnapshotAsync(LocatorAriaSnapshotOptions? options = null) + public Task AriaSnapshotAsync(LocatorAriaSnapshotOptions? options = null) => _frame.WrapApiCallAsync(async () => { var result = await _frame.SendMessageToServerAsync( "ariaSnapshot", @@ -714,9 +720,9 @@ public async Task AriaSnapshotAsync(LocatorAriaSnapshotOptions? options }, timeout: _frame.Timeout(options?.Timeout)).ConfigureAwait(false); return result!.Value.GetProperty("snapshot").ToString(); - } + }); - public async Task NormalizeAsync() + public Task NormalizeAsync() => _frame.WrapApiCallAsync(async () => { var result = await _frame.SendMessageToServerAsync("resolveSelector", new Dictionary { @@ -730,7 +736,7 @@ public async Task NormalizeAsync() locator._stringRepresentation = locatorCode.GetString(); } return locator; - } + }); } internal class ByRoleOptions diff --git a/src/Playwright/Core/Mouse.cs b/src/Playwright/Core/Mouse.cs index 5c6de778a6..409bff427e 100644 --- a/src/Playwright/Core/Mouse.cs +++ b/src/Playwright/Core/Mouse.cs @@ -48,7 +48,7 @@ public Task ClickAsync(float x, float y, MouseClickOptions? options = default) ["clickCount"] = options?.ClickCount, }); - public async Task DblClickAsync(float x, float y, MouseDblClickOptions? options = default) + public Task DblClickAsync(float x, float y, MouseDblClickOptions? options = default) => _page.WrapApiCallAsync(async () => { await _page.WrapApiCallAsync( async () => @@ -66,7 +66,7 @@ await _page.SendMessageToServerAsync( }, false, "Double click").ConfigureAwait(false); - } + }); public Task DownAsync(MouseDownOptions? options = default) => _page.SendMessageToServerAsync( diff --git a/src/Playwright/Core/Page.cs b/src/Playwright/Core/Page.cs index c3c8b0e7c3..602c606e7c 100644 --- a/src/Playwright/Core/Page.cs +++ b/src/Playwright/Core/Page.cs @@ -40,16 +40,16 @@ namespace Microsoft.Playwright.Core; internal class Page : ChannelOwner, IPage { - private readonly List _frames = new(); private readonly PageInitializer _initializer; - internal readonly List _workers = new(); internal readonly TimeoutSettings _timeoutSettings; private readonly List _harRouters = new(); private readonly Dictionary _locatorHandlers = new(); private readonly List _webSocketRoutes = new(); private readonly Video _video; private readonly Screencast _screencast; + internal List _workers = new(); + private List _frames = new(); private List _routes = new(); private string? _closeReason; @@ -60,7 +60,7 @@ internal Page(ChannelOwner parent, string guid, PageInitializer initializer) : b MainFrame = initializer.MainFrame; MainFrame.Page = this; - _frames.Add(MainFrame); + CopyOnWrite.Add(ref _frames, MainFrame); if (initializer.ViewportSize != null) { ViewportSize = new() { Width = initializer.ViewportSize.Width, Height = initializer.ViewportSize.Height }; @@ -78,8 +78,8 @@ internal Page(ChannelOwner parent, string guid, PageInitializer initializer) : b LocalStorage = new WebStorage(this, "local"); SessionStorage = new WebStorage(this, "session"); - Close += (_, _) => ClosedOrCrashedTcs.TrySetResult(true); - Crash += (_, _) => ClosedOrCrashedTcs.TrySetResult(true); + Close += (_, _) => _connection.Dispatcher.Complete(ClosedOrCrashedTcs, true); + Crash += (_, _) => _connection.Dispatcher.Complete(ClosedOrCrashedTcs, true); } private event EventHandler? _requestImpl; @@ -100,48 +100,48 @@ internal Page(ChannelOwner parent, string guid, PageInitializer initializer) : b public event EventHandler? Console { - add => this._consoleImpl = UpdateEventHandler("console", this._consoleImpl, value, true); - remove => this._consoleImpl = UpdateEventHandler("console", this._consoleImpl, value, false); + add => UpdateEventHandler("console", ref _consoleImpl, value, true, () => _consoleImpl != null); + remove => UpdateEventHandler("console", ref _consoleImpl, value, false, () => _consoleImpl != null); } public event EventHandler? Popup; public event EventHandler? Request { - add => this._requestImpl = UpdateEventHandler("request", this._requestImpl, value, true); - remove => this._requestImpl = UpdateEventHandler("request", this._requestImpl, value, false); + add => UpdateEventHandler("request", ref _requestImpl, value, true, () => _requestImpl != null); + remove => UpdateEventHandler("request", ref _requestImpl, value, false, () => _requestImpl != null); } public event EventHandler? WebSocket; public event EventHandler? Response { - add => this._responseImpl = UpdateEventHandler("response", this._responseImpl, value, true); - remove => this._responseImpl = UpdateEventHandler("response", this._responseImpl, value, false); + add => UpdateEventHandler("response", ref _responseImpl, value, true, () => _responseImpl != null); + remove => UpdateEventHandler("response", ref _responseImpl, value, false, () => _responseImpl != null); } public event EventHandler? RequestFinished { - add => this._requestFinishedImpl = UpdateEventHandler("requestFinished", this._requestFinishedImpl, value, true); - remove => this._requestFinishedImpl = UpdateEventHandler("requestFinished", this._requestFinishedImpl, value, false); + add => UpdateEventHandler("requestFinished", ref _requestFinishedImpl, value, true, () => _requestFinishedImpl != null); + remove => UpdateEventHandler("requestFinished", ref _requestFinishedImpl, value, false, () => _requestFinishedImpl != null); } public event EventHandler? RequestFailed { - add => this._requestFailedImpl = UpdateEventHandler("requestFailed", this._requestFailedImpl, value, true); - remove => this._requestFailedImpl = UpdateEventHandler("requestFailed", this._requestFailedImpl, value, false); + add => UpdateEventHandler("requestFailed", ref _requestFailedImpl, value, true, () => _requestFailedImpl != null); + remove => UpdateEventHandler("requestFailed", ref _requestFailedImpl, value, false, () => _requestFailedImpl != null); } public event EventHandler? Dialog { - add => this._dialogImpl = UpdateEventHandler("dialog", this._dialogImpl, value, true); - remove => this._dialogImpl = UpdateEventHandler("dialog", this._dialogImpl, value, false); + add => UpdateEventHandler("dialog", ref _dialogImpl, value, true, () => _dialogImpl != null); + remove => UpdateEventHandler("dialog", ref _dialogImpl, value, false, () => _dialogImpl != null); } public event EventHandler? DialogClosed { - add => this._dialogClosedImpl = UpdateEventHandler("dialogClosed", this._dialogClosedImpl, value, true); - remove => this._dialogClosedImpl = UpdateEventHandler("dialogClosed", this._dialogClosedImpl, value, false); + add => UpdateEventHandler("dialogClosed", ref _dialogClosedImpl, value, true, () => _dialogClosedImpl != null); + remove => UpdateEventHandler("dialogClosed", ref _dialogClosedImpl, value, false, () => _dialogClosedImpl != null); } public event EventHandler