ginkgo v2 leaks goroutine

[cyga]

How to reproduce:

cd /tmp
mkdir gokingoleak
echo 'package gokingoleak

import (
  "testing"

  "github.com/onsi/ginkgo/v2"
  "github.com/onsi/gomega"
  "go.uber.org/goleak"
)

func TestLeak(t *testing.T) {
  defer goleak.VerifyNone(t)

  gomega.RegisterFailHandler(ginkgo.Fail)
  ginkgo.RunSpecs(t, "Test")
}' > gokingoleak_test.go
go mod init gokingoleak
go mod tidy
go test

Running Suite: Test - /tmp
==========================
Random Seed: 1709289977

Will run 0 of 0 specs

Ran 0 of 0 Specs in 0.000 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped
--- FAIL: TestLeak (0.44s)
    gokingoleak_test.go:16: found unexpected goroutines:
        [Goroutine 37 in state select, with github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts.func2 on top of the stack:
        github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts.func2(0x0?)
        	/Users/alexandr.sudakov/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:131 +0x74
        created by github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts in goroutine 35
        	/Users/alexandr.sudakov/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:128 +0x16c
        ]
FAIL
exit status 1
FAIL	gokingoleak	1.197s

It looks like the following diff can fix this problem:

diff --git a/core_dsl.go b/core_dsl.go
index 0e633f3..ed23330 100644
--- a/core_dsl.go
+++ b/core_dsl.go
@@ -297,7 +297,9 @@ func RunSpecs(t GinkgoTestingT, description string, args ...interface{}) bool {
 	suitePath, err = filepath.Abs(suitePath)
 	exitIfErr(err)
 
-	passed, hasFocusedTests := global.Suite.Run(description, suiteLabels, suitePath, global.Failer, reporter, writer, outputInterceptor, interrupt_handler.NewInterruptHandler(client), client, internal.RegisterForProgressSignal, suiteConfig)
+	interruptHandler := interrupt_handler.NewInterruptHandler(client)
+	defer interruptHandler.Stop()
+	passed, hasFocusedTests := global.Suite.Run(description, suiteLabels, suitePath, global.Failer, reporter, writer, outputInterceptor, interruptHandler, client, internal.RegisterForProgressSignal, suiteConfig)
 	outputInterceptor.Shutdown()
 
 	flagSet.ValidateDeprecations(deprecationTracker)

Though, I'm not sure if this is the best way to do it.

[onsi]

yes - there are goroutines that outlive each individual spec and that outlive the entire test. the fix you propose would fix the leak that outlives the entire test. fwiw - you may want to use gleak: https://onsi.github.io/gomega/#codegleakcode-finding-leaked-goroutines - it’s tailored to work with Ginkgo and Gomega and would help you confirm that your tests don’t leak at a more granular level (i.e. no individual spec should leak - not just the entire suite). this is useful to debug when leaks occur. it also has the advantage of being aware of ginkgo’s goroutines and ignoring them.

[cyga]

@onsi thanks for a quick response and advice about gleak. I already saw gleak mentioned in one of open issues. I am going to try to adopt it as a workaround for automatic goroutine leak testing in my company's monorepo.

However, as far as I understood there is no need for the interrupt handler goroutine to outlive the whole suite. Thus, this is a good idea to change this behaviour. Can you confirm that?

[onsi]

yes that’s also correct - if you want to submit a PR please do