Skip to content
This repository was archived by the owner on Jan 30, 2025. It is now read-only.

Commit 1f4392b

Browse files
Ivan Mirićimiric
authored andcommitted
Revert "Use context cancellation for signaling that browser process is done"
This reverts commit e35e88c. It seems the previous change might've been causing some flakiness issues. See https://github.com/grafana/xk6-browser/issues/566#issuecomment-1285672600
1 parent 87e99fe commit 1f4392b

File tree

2 files changed

+14
-15
lines changed

2 files changed

+14
-15
lines changed

common/browser.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ func (b *Browser) Close() {
429429
// forcefully after the timeout.
430430
timeout := time.Second
431431
select {
432-
case <-b.browserProc.ctx.Done():
432+
case <-b.browserProc.processDone:
433433
case <-time.After(timeout):
434434
b.logger.Debugf("Browser:Close", "killing browser process with PID %d after %s", b.browserProc.Pid(), timeout)
435435
b.browserProc.Terminate()

common/browser_process.go

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type BrowserProcess struct {
2424
// Channels for managing termination.
2525
lostConnection chan struct{}
2626
processIsGracefullyClosing chan struct{}
27+
processDone chan struct{}
2728

2829
// Browser's WebSocket URL to speak CDP
2930
wsURL string
@@ -38,26 +39,23 @@ func NewBrowserProcess(
3839
ctx context.Context, path string, args, env []string, dataDir *storage.Dir,
3940
ctxCancel context.CancelFunc, logger *log.Logger,
4041
) (*BrowserProcess, error) {
41-
procCtx, procCtxCancel := context.WithCancel(ctx)
42-
cmd, err := execute(
43-
procCtx, procCtxCancel, path, args, env, dataDir, logger)
42+
cmd, err := execute(ctx, ctxCancel, path, args, env, dataDir, logger)
4443
if err != nil {
45-
procCtxCancel()
4644
return nil, err
4745
}
4846

49-
wsURL, err := parseDevToolsURL(cmd)
47+
wsURL, err := parseDevToolsURL(ctx, cmd)
5048
if err != nil {
51-
procCtxCancel()
5249
return nil, err
5350
}
5451

5552
p := BrowserProcess{
56-
ctx: procCtx,
53+
ctx: ctx,
5754
cancel: ctxCancel,
5855
process: cmd.Process,
5956
lostConnection: make(chan struct{}),
6057
processIsGracefullyClosing: make(chan struct{}),
58+
processDone: cmd.done,
6159
wsURL: wsURL,
6260
userDataDir: dataDir,
6361
}
@@ -67,7 +65,7 @@ func NewBrowserProcess(
6765
// browser-initiated termination then cancel the context to clean up.
6866
select {
6967
case <-p.lostConnection:
70-
case <-procCtx.Done():
68+
case <-ctx.Done():
7169
}
7270

7371
select {
@@ -123,7 +121,7 @@ func (p *BrowserProcess) AttachLogger(logger *log.Logger) {
123121

124122
type command struct {
125123
*exec.Cmd
126-
ctx context.Context
124+
done chan struct{}
127125
stdout, stderr io.Reader
128126
}
129127

@@ -161,13 +159,14 @@ func execute(
161159
return command{}, fmt.Errorf("%w", ctx.Err())
162160
}
163161

162+
done := make(chan struct{})
164163
go func() {
165164
// TODO: How to handle these errors?
166165
defer func() {
167166
if err := dataDir.Cleanup(); err != nil {
168167
logger.Errorf("browser", "cleaning up the user data directory: %v", err)
169168
}
170-
ctxCancel()
169+
close(done)
171170
}()
172171

173172
if err := cmd.Wait(); err != nil {
@@ -177,11 +176,11 @@ func execute(
177176
}
178177
}()
179178

180-
return command{cmd, ctx, stdout, stderr}, nil
179+
return command{cmd, done, stdout, stderr}, nil
181180
}
182181

183182
// parseDevToolsURL grabs the websocket address from chrome's output and returns it.
184-
func parseDevToolsURL(cmd command) (wsURL string, _ error) {
183+
func parseDevToolsURL(ctx context.Context, cmd command) (wsURL string, _ error) {
185184
type result struct {
186185
devToolsURL string
187186
err error
@@ -214,7 +213,7 @@ func parseDevToolsURL(cmd command) (wsURL string, _ error) {
214213
select {
215214
case r := <-c:
216215
return r.devToolsURL, r.err
217-
case <-cmd.ctx.Done():
218-
return "", fmt.Errorf("%w", cmd.ctx.Err())
216+
case <-ctx.Done():
217+
return "", fmt.Errorf("%w", ctx.Err())
219218
}
220219
}

0 commit comments

Comments
 (0)