From 3911ab739e9849cb620cac66d64885337cc73177 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 15 Oct 2026 11:02:00 -0700 Subject: [PATCH 1/2] docs(claude): describe the threading model and client architecture Document that Core/ mirrors the JS client method for method, how messages are dispatched on the transport reader thread while user continuations run on the thread pool, and what that means for changes: subscribe before checking, no locks, event subscriptions that talk to the server, NoInlining on public API methods. Also where regression tests go, how the test adapters are structured and how they are tested. --- CLAUDE.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index eb1bd94b9..b3376a0a3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,14 @@ grep "^ Failed" /tmp/test-results.txt # list failures tail -5 /tmp/test-results.txt # summary ``` +Run a single class with `--filter "FullyQualifiedName~PageWaitForUrlTests"`. Every `dotnet test` invocation is a fresh test host, so run a timing-sensitive repro a few times, on `main` and on the fix. + +Test classes in `src/Playwright.Tests` mirror the upstream specs by name (`PageWaitForUrlTests` ↔ `tests/page/page-wait-for-url.spec.ts` in microsoft/playwright). Add a regression test to the matching class as a `[PlaywrightTest]` method; `PageTestEx` gives you `Page` and a local `Server` (`Server.SetRoute`, `Server.Prefix`, `Server.EmptyPage`). + +The test adapters are not covered by `Playwright.Tests`. They are tested from the Node harness in `src/Playwright.TestingHarnessTest`: `npm ci`, then `TEST_MODE=nunit npx playwright test nunit.spec.ts` (`TEST_MODE` is one of `nunit`, `mstest`, `mstest.v4`, `xunit`, `xunit.v3`). + +CI's style check is `dotnet format ./src/ --verify-no-changes`. + ## Architecture ### Generated vs hand-written code @@ -36,12 +44,36 @@ tail -5 /tmp/test-results.txt # summary - The generator skips generating interface files for types like `TimeoutException` and `IAsyncDisposable` that map to built-in .NET types. - Supplement interfaces (`src/Playwright/API/Supplements/`) are hand-written and extend the generated interfaces with .NET-specific overloads. - Internal implementations live in `src/Playwright/Core/` (namespace `Microsoft.Playwright.Core`). These implement both the generated and supplement interfaces. +- Protocol types in `src/Playwright/Transport/Protocol/Generated/` are generated by `../playwright/utils/generate_dotnet_channels.js`. Both generators run as part of `./build.sh --roll `; a wrong doc comment, option name or overload in the .NET API is an upstream change to `docs/src/api/` or the generator and reaches this repo with the next roll. + +### The client mirrors the JS client +- `src/Playwright/Core/*.cs` is a port of `packages/playwright-core/src/client/*.ts` in microsoft/playwright, method for method (`Frame.cs` ↔ `frame.ts`, `Waiter.cs` ↔ `waiter.ts`, `Connection.cs` ↔ `connection.ts`). Keep the same method boundaries, order of steps and log messages: reviews compare against the JS original, and rolls port upstream client commits one by one. +- Before restructuring a method, read its JS counterpart. If JS does the same thing, the change is compensating for something the .NET runtime does differently (usually threading, below); keep the deviation as small as the method allows and leave a comment naming the reason. +- If the same calls misbehave in the JS client, fix it upstream first. + +### Threading model +The JS client is single-threaded and the other ports keep that model. .NET does not: the library and the user's code run concurrently, which is where "lost event" and "collection was modified" reports come from (#2719, #3207, #3364). + +- `StdIOTransport` reads the driver's stdout on a dedicated long-running task and calls `Connection.Dispatch` for each message. `Dispatch` runs `ChannelOwner.OnMessage` synchronously on that thread, so `Frame.OnNavigated`/`OnLoadState`, the `Url` and `_loadStates` updates and every event handler user code attached (`page.FrameNavigated += ...`) run on the reader thread, and route handlers start there (until their first `await`). A handler that blocks (`.Result`, `.Wait()`) stalls all later messages, including the reply it is waiting for. +- Replies to commands complete a `TaskCompletionSource` created with `RunContinuationsAsynchronously`, so code after `await page.GotoAsync(...)` runs on a thread-pool thread while the reader keeps dispatching. JS guarantees that a continuation runs before the next message is dispatched; .NET does not. +- Outgoing messages are serialized through `Connection._queue` (a `TaskQueue` over a `SemaphoreSlim`). Incoming messages are not synchronized with user code. +- Sharing one `IPlaywright` across threads is not supported (#2560); use one instance per thread. + +Consequences for changes: +- "Check state, then subscribe to the event" is correct in JS and a race here: the reader thread can deliver the event between the check and the subscription, and the wait times out (#3364). Subscribe first, then check, and unsubscribe when the check already passes. +- Do not add `lock`. The client has none. For state the reader thread mutates and user code reads, take a snapshot (`_frames.ToList()` in #2724, `_contextsForSelectors.ToArray()` in #3208) or use a concurrent collection (`ConcurrentDictionary` in `RouteHandler`, #2887). +- Adding or removing an event handler can send `updateSubscription` to the server: `ChannelOwner.UpdateEventHandler` does so for the first and last handler of `console`, `dialog`, `fileChooser`, `request`, `response`, `requestFinished` and `requestFailed`. `Waiter` subscribes through reflection (`GetEvent` + `AddEventHandler`). ### Key patterns - All Playwright objects extend `ChannelOwner` and communicate via `SendMessageToServerAsync`. - `Connection.cs` has a factory switch that creates the right `ChannelOwner` subclass based on `ChannelOwnerType`. - 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. +- Public API methods in `Core/` carry `[MethodImpl(MethodImplOptions.NoInlining)]`. `Connection.WrapApiCallAsync` walks the stack once per outermost API call to attach the user's frame to call logs and traces, and inlining breaks that walk (#2620). Keep the attribute on new public methods. + +### Test adapters +- `Playwright.NUnit`, `Playwright.MSTest`, `Playwright.MSTest.v4`, `Playwright.Xunit` and `Playwright.Xunit.v3` each repeat the same hierarchy: `WorkerAwareTest` → `PlaywrightTest` → `BrowserTest` → `ContextTest` → `PageTest`. A change to one usually belongs in all five. +- Settings come from `.runsettings` through `Playwright.TestAdapter`: `PlaywrightSettingsProvider` is a VSTest `ISettingsProvider` for the `` section, and `PlaywrightSettingsXml` maps the XML onto the option classes by reflection. Microsoft.Testing.Platform does not run VSTest settings providers, which is behind most "runsettings are ignored" reports. ## Commit Convention From 87751a752a86f833ecbbf811dafcc19768268daa Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Thu, 15 Oct 2026 18:01:40 -0700 Subject: [PATCH 2/2] docs(claude): describe message ordering and inline event continuations Incoming messages are handled one at a time, so an event that precedes a reply is fully handled before the awaiting code resumes, while events after the reply race with it. Waits that complete through an event resume inline on the reader thread. --- CLAUDE.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b3376a0a3..050090100 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,13 +54,16 @@ CI's style check is `dotnet format ./src/ --verify-no-changes`. ### Threading model The JS client is single-threaded and the other ports keep that model. .NET does not: the library and the user's code run concurrently, which is where "lost event" and "collection was modified" reports come from (#2719, #3207, #3364). -- `StdIOTransport` reads the driver's stdout on a dedicated long-running task and calls `Connection.Dispatch` for each message. `Dispatch` runs `ChannelOwner.OnMessage` synchronously on that thread, so `Frame.OnNavigated`/`OnLoadState`, the `Url` and `_loadStates` updates and every event handler user code attached (`page.FrameNavigated += ...`) run on the reader thread, and route handlers start there (until their first `await`). A handler that blocks (`.Result`, `.Wait()`) stalls all later messages, including the reply it is waiting for. -- Replies to commands complete a `TaskCompletionSource` created with `RunContinuationsAsynchronously`, so code after `await page.GotoAsync(...)` runs on a thread-pool thread while the reader keeps dispatching. JS guarantees that a continuation runs before the next message is dispatched; .NET does not. +- Incoming messages are handled one at a time, in protocol order. `StdIOTransport.GetResponseAsync` reads the driver's stdout and calls `Connection.Dispatch` for each message; `Dispatch` runs `ChannelOwner.OnMessage` synchronously, so `Frame.OnNavigated`/`OnLoadState`, the `Url` and `_loadStates` updates and every event handler user code attached (`page.FrameNavigated += ...`) have finished before the next message is looked at. There is no fixed reader thread: the read loop is started as a long-running task but continues on whatever thread-pool thread completes each pipe read, so "the reader thread" below is a role, not a thread id. +- Replies to commands complete a `TaskCompletionSource` created with `RunContinuationsAsynchronously` (`Connection.cs`), which queues the awaiting code to the thread pool. Everything the reader did for messages *before* the reply is visible when `await page.GotoAsync(...)` returns: an event that precedes the reply in the protocol can never fire after the method has returned, and `GotoAsync` can rely on `_loadStates` containing `load`. Events that *follow* the reply race with the continuation. JS guarantees that a continuation runs before the next message is dispatched; .NET does not. +- Waits that complete through an event resume *inline on the reader thread*: `Waiter.GetWaitForEventTask` completes a plain `TaskCompletionSource`, so after `await page.WaitForLoadStateAsync()`, `WaitForNavigationAsync`, `WaitForURLAsync`, `WaitForEventAsync` or `RunAndWaitFor*Async` (when the event has not already happened) code runs inside `Connection.Dispatch` until its next incomplete `await`. Blocking there (`.Result`, `.Wait()`) stalls every later message, including the reply being waited for. It also inverts an ordering JS has: `Frame.OnLoadState` raises `Frame.LoadState` before `Page.Load`, so right after `await page.WaitForLoadStateAsync()` a `page.Load` handler has not run yet. +- A few messages are handed off on purpose and run concurrently with later messages: `route` and `webSocketRoute` (`_ = OnRouteAsync(...)`), `bindingCall`, locator handlers and screencast frames. Their handlers start on the reader thread and move to the pool at their first incomplete `await`; they have to call back into the server, so they cannot run inline. - Outgoing messages are serialized through `Connection._queue` (a `TaskQueue` over a `SemaphoreSlim`). Incoming messages are not synchronized with user code. - Sharing one `IPlaywright` across threads is not supported (#2560); use one instance per thread. Consequences for changes: - "Check state, then subscribe to the event" is correct in JS and a race here: the reader thread can deliver the event between the check and the subscription, and the wait times out (#3364). Subscribe first, then check, and unsubscribe when the check already passes. +- A method may rely on state produced by events that precede its reply in the protocol, never on events that arrive after it. Library code that runs after an event-driven wait may be on the reader thread: it must not block, and must not assume that later events of the same dispatch (`Page.Load` after `Frame.LoadState`) have fired. - Do not add `lock`. The client has none. For state the reader thread mutates and user code reads, take a snapshot (`_frames.ToList()` in #2724, `_contextsForSelectors.ToArray()` in #3208) or use a concurrent collection (`ConcurrentDictionary` in `RouteHandler`, #2887). - Adding or removing an event handler can send `updateSubscription` to the server: `ChannelOwner.UpdateEventHandler` does so for the first and last handler of `console`, `dialog`, `fileChooser`, `request`, `response`, `requestFinished` and `requestFailed`. `Waiter` subscribes through reflection (`GetEvent` + `AddEventHandler`).