-
-
Notifications
You must be signed in to change notification settings - Fork 441
Description
Rod Version: v0.116.2
Description
Page.cleanupStates() only removes the cached *Page entry (keyed by TargetID) from browser.states, but does not remove the per-session CDP method states that were accumulated by Browser.set() during the page's lifetime.
Every Browser.Call() invocation stores its params in browser.states (a sync.Map) keyed by stateKey{browserContextID, sessionID, methodName}. When a page is closed, these entries are never deleted. For long-lived browsers with many page open/close cycles, this is an unbounded memory leak.
Root Cause
Browser.Call() → Browser.set() stores params forever
// browser.go
func (b *Browser) Call(ctx context.Context, sessionID, methodName string, params interface{}) (res []byte, err error) {
res, err = b.client.Call(ctx, sessionID, methodName, params)
if err != nil {
return nil, err
}
b.set(proto.TargetSessionID(sessionID), methodName, params) // ← stores params permanently
return
}Page.cleanupStates() only removes the TargetID entry
// states.go
func (p *Page) cleanupStates() {
p.browser.RemoveState(p.TargetID) // Only removes the cached *Page entry
}This removes the TargetID → *Page entry stored by cachePage(), but none of the stateKey → params entries stored by set().
Why it leaks
Each page has a unique SessionID. Over the browser's lifetime:
- Page is created → gets a
SessionID - CDP calls are made →
stateKey{..., sessionID, method}entries accumulate inbrowser.states - Page is closed →
cleanupStates()only removes theTargetIDentry - The
stateKeyentries remain forever because no code iterates the map to find entries matching the closed page'sSessionID
Impact
- Any CDP call with large params (e.g.,
Page.SetDocumentContentwith multi-MB HTML) permanently leaks the params struct - Affects any application using a long-lived
*rod.Browserwith many page open/close cycles - Severity scales with the number and size of CDP calls made per page
Code to Reproduce
package main
import (
"fmt"
"runtime"
"strings"
"github.com/go-rod/rod"
)
func main() {
b := rod.New().MustConnect()
defer b.MustClose()
printMemStats("before")
for i := 0; i < 500; i++ {
page := b.MustPage("about:blank")
page.MustSetDocumentContent("<html><body>" + strings.Repeat("x", 1024*1024) + "</body></html>")
page.MustClose()
}
runtime.GC()
printMemStats("after 500 page cycles")
}
func printMemStats(label string) {
var m runtime.MemStats
runtime.ReadMemStats(&m)
fmt.Printf("[%s] HeapInuse: %d MB\n", label, m.HeapInuse/1024/1024)
}Expected: heap stays roughly constant after pages are closed.
Actual: heap grows ~1 MB per iteration (the SetDocumentContent params are never freed).
Suggested Fix
Update cleanupStates() to iterate browser.states and delete all entries keyed by stateKey matching the closing page's SessionID:
func (p *Page) cleanupStates() {
p.browser.RemoveState(p.TargetID)
p.browser.states.Range(func(key, _ interface{}) bool {
if k, ok := key.(stateKey); ok && k.sessionID == p.SessionID {
p.browser.states.Delete(key)
}
return true
})
}sync.Map.Range is documented as safe for concurrent Store/Delete during iteration. This is O(n) over the states map on each page close, but n is small for typical usage (few concurrent pages).
I'm happy to submit a PR with this fix and tests if the approach looks good.
Related
- memory leak by use HijackRequests() #748 — reported the same symptom (memory grows with repeated page open/close on a long-lived browser). The workaround of periodically closing and re-creating the
Browseris consistent with this root cause, since closing the browser clears the entirestatesmap.