From ea9f291b8afcc4ce9b9725aca9c3851811e92107 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 2 Aug 2023 12:27:10 +0100 Subject: [PATCH 1/2] Add unsubscribe in handleExitEvent We've found that when we perform a e2e test in k6 that the goroutines handling the handleIterEvents do not get cleared away. This is due to us not unsubscribing when the test exits. --- browser/registry.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/browser/registry.go b/browser/registry.go index b4eb28386..511e26544 100644 --- a/browser/registry.go +++ b/browser/registry.go @@ -213,8 +213,6 @@ func newBrowserRegistry(vu k6modules.VU, remote *remoteRegistry, pids *pidRegist exitSubID, exitCh := vu.Events().Global.Subscribe( k6event.Exit, ) - go r.handleExitEvent(exitCh) - iterSubID, eventsCh := vu.Events().Local.Subscribe( k6event.IterStart, k6event.IterEnd, @@ -223,6 +221,8 @@ func newBrowserRegistry(vu k6modules.VU, remote *remoteRegistry, pids *pidRegist vu.Events().Local.Unsubscribe(iterSubID) vu.Events().Global.Unsubscribe(exitSubID) } + + go r.handleExitEvent(exitCh, unsubscribe) go r.handleIterEvents(eventsCh, unsubscribe) return r @@ -282,7 +282,9 @@ func (r *browserRegistry) handleIterEvents(eventsCh <-chan *k6event.Event, unsub } } -func (r *browserRegistry) handleExitEvent(exitCh <-chan *k6event.Event) { +func (r *browserRegistry) handleExitEvent(exitCh <-chan *k6event.Event, unsubscribeFn func()) { + defer unsubscribeFn() + e, ok := <-exitCh if !ok { return From 642bc7db9a24162808c4cee36d7a1b2700c85425 Mon Sep 17 00:00:00 2001 From: ankur22 Date: Wed, 2 Aug 2023 12:41:52 +0100 Subject: [PATCH 2/2] Remove goroutine datadir cleanup This is no longer required since we're cleaning up the datadir when we call browser.close(). --- chromium/browser_type.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/chromium/browser_type.go b/chromium/browser_type.go index 33366a2ad..a994fc753 100644 --- a/chromium/browser_type.go +++ b/chromium/browser_type.go @@ -185,20 +185,6 @@ func (b *BrowserType) launch( } flags["user-data-dir"] = dataDir.Dir - go func(c context.Context) { - defer func() { - if err := dataDir.Cleanup(); err != nil { - logger.Errorf("BrowserType:Launch", "cleaning up the user data directory: %v", err) - } - }() - // There's a small chance that this might be called - // if the context is closed by the k6 runtime. To - // guarantee the cleanup we would need to orchestrate - // it correctly which https://github.com/grafana/k6/issues/2432 - // will enable once it's complete. - <-c.Done() - }(ctx) - browserProc, err := b.allocate(ctx, opts, flags, dataDir, logger) if browserProc == nil { return nil, 0, fmt.Errorf("launching browser: %w", err)