Data race in Context.Target between NewContext's cancellation watcher and attachTarget
Summary
chromedp.NewContext's internal cancellation-watcher goroutine reads Context.Target with no synchronization, while attachTarget (reached via chromedp.Run -> newTarget) writes that same field, also with no synchronization. If the ancestor context a tab's context was derived from is cancelled while attachTarget is in flight for that tab, for example because the browser process died, the two race. Confirmed against chromedp v0.16.0, the newest tagged release at the time of writing (no newer version exists to test against).
Environment
chromedp v0.16.0. go1.26.5, windows/amd64. Reproduced consistently: 3/3 runs under go test -race. Not yet tested on Linux, happy to check if that's useful.
Where the two accesses are
The read, in chromedp.go inside NewContext:
go func() {
<-ctx.Done()
defer c.closedTarget.Done()
if c.first {
return
}
if c.Target == nil { // unsynchronized read
return
}
...
}()The write, in chromedp.go inside attachTarget, called from newTarget, called from Run:
func (c *Context) attachTarget(ctx context.Context, targetID target.ID) error {
sessionID, err := target.AttachToTarget(targetID).WithFlatten(true).Do(cdp.WithExecutor(ctx, c.Browser))
if err != nil {
return err
}
c.Target, err = c.Browser.newExecutorForTarget(ctx, targetID, sessionID) // unsynchronized write
...
}How to reproduce
A minimal reproduction shape, adapted from our actual failing test:
Allocate a browser with chromedp.NewContext(chromedp.NewExecAllocator(...)) and keep its cancel function.
In a goroutine, after a short delay (we used 300ms), call that cancel function directly, standing in for "the browser process crashed" (in production this happens via allocate.go's Browser.LostConnection watcher instead, but a direct cancel reproduces the same race since the effect on Context.Target synchronization is identical).
Concurrently, on the main goroutine, derive a new tab context from the browser context with chromedp.NewContext(browserCtx) and call chromedp.Run(tabCtx, someSlowAction), an action slow enough that the kill in step 2 has a real chance of landing while attachTarget is still executing.
Run under go test -race. It fails with WARNING: DATA RACE naming chromedp.go:182 (the read) and chromedp.go:434 (the write), or occasionally browser.go:158's newExecutorForTarget and chromedp.go:193 for a second, related field.
Full race output (one representative run)
==================
WARNING: DATA RACE
Read at 0x00c0002997a8 by goroutine 35:
github.com/chromedp/chromedp.NewContext.func1()
chromedp.go:182 +0x116
Previous write at 0x00c0002997a8 by goroutine 9:
github.com/chromedp/chromedp.(*Context).attachTarget()
chromedp.go:434 +0x284
github.com/chromedp/chromedp.(*Context).newTarget()
chromedp.go:382 +0xd24
github.com/chromedp/chromedp.Run()
chromedp.go:331 +0xa5
[caller frames omitted, see repro steps above]
Goroutine 35 (running) created at:
github.com/chromedp/chromedp.NewContext()
chromedp.go:173 +0x65b
[caller frames omitted]
Goroutine 9 (running) created at:
testing.(*T).Run()
testing.go:2101 +0xb2a
[test harness frames omitted]
==================
==================
WARNING: DATA RACE
Read at 0x00c0001368c8 by goroutine 35:
github.com/chromedp/chromedp.NewContext.func1()
chromedp.go:193 +0x204
Previous write at 0x00c0001368c8 by goroutine 9:
github.com/chromedp/chromedp.(*Browser).newExecutorForTarget()
browser.go:158 +0x134
github.com/chromedp/chromedp.(*Context).attachTarget()
chromedp.go:434 +0x25d
[same call chain as above]
==================What we verified before concluding this is real
Both racing stacks descend from the same single call to our own wrapper around NewContext + Run. This is not two concurrent renders racing each other, and not a retry-loop leftover-goroutine issue. NewContext's own returned cancel function (cancelWait) already correctly blocks on c.closedTarget.Wait(), so a previous attempt's watcher goroutine is confirmed torn down before a caller's next attempt begins. The race is within one attempt, between its own NewContext watcher and its own attachTarget.
We added a check immediately before calling NewContext/Run, rejecting a render early if the browser context was already Err() != nil at that point. This is a correct narrowing (it protects the case of a browser already known dead before a render starts), but, verified over 3 further -race runs, it does NOT stop this race: the browser can still die in the interval between that check and attachTarget completing, which is the exact scenario our test constructs and which real Chrome crashes can equally land in.
No newer chromedp release exists to test against (go list -m -versions shows v0.16.0 as latest).
Suggested fix
A sync.Mutex (or atomic.Pointer[Target]) guarding reads and writes of Context.Target would close this without changing chromedp's public API. The watcher goroutine's if c.Target == nil { return } check and attachTarget's c.Target, err = ... assignment are the two sites that need to agree on the same lock.
Happy to submit a minimal standalone repro (independent of our own codebase) if that would help - let me know.
Source: chromedp/chromedp