Repository navigation
test(v3/windows): keep the WebView2 lifetime tests valid under checkptr (-race) - #6348
Conversation
go test -race enables checkptr, which aborts the process when a uintptr is converted back into a pointer into the Go heap or stack without a pointer in the same expression to derive it from. The edge tests that stand in for native code made that conversion: they held a handler as a bare address, as WebView2 does, and turned it back into a pointer to read its vtable or look up its reference count. The QueryInterface stubs did the same with their out-parameter. Under -race the package dies with "checkptr: pointer arithmetic result points to invalid allocation" in TestLiveDelayedControllerCallbackAfterAbandon. A native reference is now a comRef: the address as an integer, which still does not keep the handler alive, and the vtable read from the handler while it was reachable. AddRef and Release call through that vtable, reference counts are matched by address, and the abandoned controller test captures the handler's Invoke slot before dropping it. The QueryInterface stubs take the out-parameter as a typed pointer, as the handlers' own callbacks take this. What the tests verify is unchanged, and no production code changes.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
COM reference tracking and abandoned-handler tests v3/internal/webview2/pkg/edge/chromium_lifetime_test.go, v3/internal/webview2/pkg/edge/com_refs_test.go |
comRef stores the handler address and vtable. Its methods access reference counts and call AddRef and Release through the vtable. The tests use comRef to inspect handler counts and test weakly referenced window collection. The abandoned-handler test captures and invokes the handler’s Invoke procedure. |
Lifetime test adoption and typed mocks v3/internal/webview2/pkg/edge/chromium_lifetime_live_test.go, v3/internal/webview2/pkg/edge/chromium_recovery_test.go, v3/internal/webview2/pkg/edge/chromium_runtime_errors_test.go |
Live lifetime tests use comRef to track handlers and check reference counts. The recovery and runtime-error tests assign controllers through typed QueryInterface output parameters. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: leaanthony, taliesin-ai
Merge Risk: 
16ef0
-
The lifetime tests retain handlers while native references remain outstanding. No merge-blocking risk is identified.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely identifies the Windows WebView2 lifetime test fix for checkptr and race-enabled testing. Description check The description explains the failure, motivation, implementation, test scope, environment, and checklist status. It is sufficiently complete for a test-only change, although it does not provide a link… Docstring Coverage Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the failure, motivation, implementation, test scope, environment, and checklist status. It is sufficiently complete for a test-only change, although it does not provide a linked issue number or explicit dependency statement.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each handler’s hold,
And counts the refs without pointers old.
Typed outputs pass their value through,
Weak windows fade from every view.
The test burrow rests, its checks complete.
…ion batches The WebView2 lifetime tests prove that an abandoned Chromium no longer retains its window by waiting for a weak pointer to the window to become nil. Go does not promise that for a tiny pointer-free object: the tiny allocator batches such objects into one allocation slot, and a weak pointer into a batch stays non-nil while any object in it is referenced. hostWindow was a single int, so without -race (which disables the tiny allocator) it could share a slot with an allocation the replacement Chromium keeps alive. TestLiveReplacementReleasesAbandonedHandlers then failed intermittently with "abandoned Chromium retained its window". hostWindow carries a pointer field, which keeps it out of any batch. TestHostWindowIsCollectedBesideLiveObjects allocates windows beside live pointer-free neighbours and requires each to be collected; it fails on every run without the field. No production code changes.
|
Amazing! Thank you! |
Description
go test -race ./internal/webview2/pkg/edge/on Windows aborts in the handler-lifetime tests added in #6184 and #6222.-raceenables checkptr, which rejects turning auintptrback into a Go pointer, and those tests rebuild handler pointers from the addresses they hold. CI runs the Windows tests without-race, so this has gone unnoticed, but anyone running the package with-raceor-d=checkptrhits it.This is a test-only change. The tests stand in for native code and now hold a handler the way native code does:
uintptr, which keeps nothing alive) plus the vtable read while the handler was reachable, and callAddRef/Releasethrough that;None of them converts an integer back into a pointer any more. Each test asserts the same things as before. The
moveFocusRequestedhandler added in #6306 is covered the same way as the others.There is no production change, so there is no changelog entry.
Type of change
How Has This Been Tested?
go test -race ./internal/webview2/pkg/edge/aborts with a checkptr failure onmasterand passes with this change (GitHub Actionswindows-latest, under-race);go test ./internal/webview2/pkg/edge/passes as before.GOOS=windows go vetandGOOS=windows go test -cpass for amd64, arm64 and 386.Test Configuration
Built from source at
masterb6167d9 + this commit, Go 1.26.3.Checklist:
Summary by CodeRabbit