Skip to content

Avoid caching helpers for reset contexts - #1240

Closed
yhuikzdtguioaert wants to merge 1 commit into
go-rod:mainfrom
yhuikzdtguioaert:fix/helper-context-reset
Closed

yhuikzdtguioaert wants to merge 1 commit into
go-rod:mainfrom
yhuikzdtguioaert:fix/helper-context-reset

Conversation

@yhuikzdtguioaert

Copy link
Copy Markdown

Summary

  • avoid writing a helper into a cache entry removed by a concurrent JavaScript context reset
  • leave the stale helper uncached so the active context can ensure its own helper
  • add deterministic coverage for whole-cache reset, single-context reset, and the normal unchanged-context path

Fixes #707.

Testing

  • go test -count=20 -run TestPageSetHelperAfterContextReset .
  • go vet ./...
  • gofumpt -d page_eval.go page_eval_internal_test.go
  • npx -ys -- cspell@6.31.1 --no-progress page_eval.go page_eval_internal_test.go
  • fork CI: Windows full test job passed; Go 1.18 build job passed

The fork's Linux job stopped in go generate because Chromium could not start with the hosted runner sandbox, and its macOS job stopped before tests with a dyld LC_UUID runner/toolchain error.

A navigation can reset a JavaScript context while its helper is being created. In that case setHelper currently indexes a cache entry that has already been removed and panics.

Only cache the helper when the context entry still exists, and cover full, per-context, and unchanged cache states.
@yhuikzdtguioaert yhuikzdtguioaert closed this by deleting the head repository Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should check jsCtxID in rod.Page.setHelper

1 participant