Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix nil pointer panic on Frame.networkIdleCtx #118

Merged
merged 2 commits into from
Nov 18, 2021
Merged

Fix nil pointer panic on Frame.networkIdleCtx #118

merged 2 commits into from
Nov 18, 2021

Commits on Nov 18, 2021

  1. Fix nil pointer panic on Frame.networkIdleCtx 

    For very quick requests there's a race condition between when we set
`f.networkIdleCtx` to nil[1] as part of the `FrameManager.requestStarted()`
call, and when we start the idle timer again in `FrameManager.requestFinished()`[2].

This fix ensures we keep the reference to `networkIdleCtx` as part of the closure
in the goroutine crated in `Frame.startNetworkIdleTimer()`, and avoid
accessing `f.networkIdleCtx` directly.

This could use better synchronization, but this fixed it consistently for
me by testing with `examples/getattribute.js`, which also reproduces the
issue (see https://github.com/grafana/xk6-browser/runs/4225921786).

[1]: https://github.com/grafana/xk6-browser/blob/baaf58caef73370f1659c30b3d3a989ddcd8da27/common/frame.go#L273

[2]: https://github.com/grafana/xk6-browser/blob/baaf58caef73370f1659c30b3d3a989ddcd8da27/common/frame.go#L290

Fixes #109
    Ivan Mirić committed Nov 18, 2021
    Configuration menu
    Copy the full SHA
    5c7fc71 View commit details
    Browse the repository at this point in the history

  2. Use channel instead of Context for network idle timer 

    This avoids the need for nil checks and the mutex, so should be race free.
    Ivan Mirić committed Nov 18, 2021
    Configuration menu
    Copy the full SHA
    fa39ea5 View commit details
    Browse the repository at this point in the history