Skip to content

Conversation

@buraindo
Copy link
Contributor

What does this PR do?

Fixes termSignal variable override which led to goroutine leak in reaper.

Why is it important?

Without this change certain scenarios would lead to goroutine leak.

Related issues

How to test this PR

Use a test from #3260:

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

	net, err := network.New(t.Context())
	require.NoError(t, err)

	req := testcontainers.ContainerRequest{
		Image:        "redis:latest",
		ExposedPorts: []string{"6379/tcp"},
		WaitingFor:   wait.ForLog("Ready to accept connections"),
		Networks:     []string{net.Name},
	}
	redisC, err := testcontainers.GenericContainer(t.Context(), testcontainers.GenericContainerRequest{
		ContainerRequest: req,
		Started:          true,
	})
	require.NoError(t, err)
	require.NoError(t, testcontainers.TerminateContainer(redisC))
	require.NoError(t, net.Remove(t.Context()))
}

I can also commit this test to the PR, if I am allowed to add goleak dependency to go.mod.

@buraindo buraindo requested a review from a team as a code owner August 17, 2025 06:44
@netlify
Copy link

netlify bot commented Aug 17, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit d048f81
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68b976ebec2b200008a4d9cd
😎 Deploy Preview https://deploy-preview-3261--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mdelapenya mdelapenya self-assigned this Sep 4, 2025
@mdelapenya mdelapenya added the bug An issue with the library label Sep 4, 2025
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to reproduce the leak with the provided test case, so thank you very much for detecting and resolving it.

LGTM, thanks!

@mdelapenya mdelapenya merged commit bac5180 into testcontainers:main Sep 4, 2025
209 checks passed
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 5, 2025
* main:
  fix(reaper): remove termSignal override (testcontainers#3261)
  chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (testcontainers#3274)
  chore(release): refine release script to update inter-module dependencies (testcontainers#3273)
  fix(registry): update `WithHtpasswd` to use `os.CreateTemp` instead of `os.Create` with `filepath.Join`. (testcontainers#3272)
mdelapenya added a commit to knqyf263/testcontainers-go that referenced this pull request Sep 5, 2025
* main: (33 commits)
  feat(registry): add helper functions to pull and tag images (testcontainers#3275)
  fix(reaper): remove termSignal override (testcontainers#3261)
  chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (testcontainers#3274)
  chore(release): refine release script to update inter-module dependencies (testcontainers#3273)
  fix(registry): update `WithHtpasswd` to use `os.CreateTemp` instead of `os.Create` with `filepath.Join`. (testcontainers#3272)
  chore(deps): bump github.com/docker/docker from 28.2.2+incompatible to 28.3.3+incompatible (testcontainers#3270)
  chore(postgres): use require.NotNil instead of assert.NotNil (testcontainers#3252)
  fix(nats): use wait for listening port instead of wait for log (testcontainers#3256)
  chore(deps): bump github.com/go-viper/mapstructure/v2 (testcontainers#3267)
  fix(postgres): snapshot restore (testcontainers#3264)
  fix(dockermcpgateway): use duckduckgo instead of brave (testcontainers#3247)
  feat: add Solace pubsub+ module (testcontainers#3230)
  feat(options): add WithProvider (testcontainers#3241)
  chore(deps): bump github/codeql-action from 3.29.2 to 3.29.3 (testcontainers#3237)
  chore(deps): bump golang.org/x/oauth2 in /modules/weaviate (testcontainers#3240)
  chore(deps): bump mkdocs-include-markdown-plugin from 7.1.5 to 7.1.6 (testcontainers#3239)
  chore(deps): bump requests from 2.32.0 to 2.32.4 (testcontainers#3204)
  feat(mcpgateway): add MCP gateway module (testcontainers#3232)
  chore(deps): bump golang.org/x/oauth2 in /modules/pulsar (testcontainers#3236)
  chore(deps): bump golang.org/x/oauth2 in /modules/gcloud (testcontainers#3235)
  ...
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Sep 5, 2025
* main: (30 commits)
  fix: preserve unix socket schema in testcontainers host from properties (testcontainers#3213)
  feat(registry): add helper functions to pull and tag images (testcontainers#3275)
  fix(reaper): remove termSignal override (testcontainers#3261)
  chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (testcontainers#3274)
  chore(release): refine release script to update inter-module dependencies (testcontainers#3273)
  fix(registry): update `WithHtpasswd` to use `os.CreateTemp` instead of `os.Create` with `filepath.Join`. (testcontainers#3272)
  chore(deps): bump github.com/docker/docker from 28.2.2+incompatible to 28.3.3+incompatible (testcontainers#3270)
  chore(postgres): use require.NotNil instead of assert.NotNil (testcontainers#3252)
  fix(nats): use wait for listening port instead of wait for log (testcontainers#3256)
  chore(deps): bump github.com/go-viper/mapstructure/v2 (testcontainers#3267)
  fix(postgres): snapshot restore (testcontainers#3264)
  fix(dockermcpgateway): use duckduckgo instead of brave (testcontainers#3247)
  feat: add Solace pubsub+ module (testcontainers#3230)
  feat(options): add WithProvider (testcontainers#3241)
  chore(deps): bump github/codeql-action from 3.29.2 to 3.29.3 (testcontainers#3237)
  chore(deps): bump golang.org/x/oauth2 in /modules/weaviate (testcontainers#3240)
  chore(deps): bump mkdocs-include-markdown-plugin from 7.1.5 to 7.1.6 (testcontainers#3239)
  chore(deps): bump requests from 2.32.0 to 2.32.4 (testcontainers#3204)
  feat(mcpgateway): add MCP gateway module (testcontainers#3232)
  chore(deps): bump golang.org/x/oauth2 in /modules/pulsar (testcontainers#3236)
  ...
mdelapenya added a commit that referenced this pull request Sep 15, 2025
* main: (24 commits)
  chore(deps): bump golang.org/x/sys from 0.32.0 to 0.36.0 (#3282)
  feat(redpanda): add support for http proxy (#3258)
  chore(deps): bump github/codeql-action from 3.29.3 to 3.30.3 (#3287)
  chore(go): bump to Go 1.24 as minimal version (#3298)
  deps(mongodb): update MongoDB Go Driver to v2 (#3278)
  chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.5 to 4.25.6 (#3224)
  chore(deps): bump mkdocs-include-markdown-plugin from 7.1.6 to 7.1.7 (#3284)
  docs: clarify no client SDKs in production modules/images, in contributing.md (#3279)
  chore(deps): bump github.com/docker/go-connections from 0.5.0 to 0.6.0 (#3285)
  chore(deps): bump tj-actions/changed-files from 46.0.3 to 47.0.0 (#3283)
  chore(modulegen): detect missing project files after new module creation (#3281)
  chore(deps): bump github.com/docker/docker in /modules/nebulagraph (#3277)
  feat(nebulagraph): add NebulaGraph module (#3266)
  fix: preserve unix socket schema in testcontainers host from properties (#3213)
  feat(registry): add helper functions to pull and tag images (#3275)
  fix(reaper): remove termSignal override (#3261)
  chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (#3274)
  chore(release): refine release script to update inter-module dependencies (#3273)
  fix(registry): update `WithHtpasswd` to use `os.CreateTemp` instead of `os.Create` with `filepath.Join`. (#3272)
  chore(deps): bump github.com/docker/docker from 28.2.2+incompatible to 28.3.3+incompatible (#3270)
  ...
mdelapenya added a commit to prestonvasquez/testcontainers-go that referenced this pull request Sep 17, 2025
* main: (22 commits)
  chore(deps): bump golang.org/x/net from 0.28.0 to 0.38.0 (testcontainers#3299)
  feat: allow saving specific platforms for an image (testcontainers#3218)
  chore(deps): bump dario.cat/mergo from 1.0.1 to 1.0.2 (testcontainers#3238)
  chore(deps): bump golang.org/x/sys from 0.32.0 to 0.36.0 (testcontainers#3282)
  feat(redpanda): add support for http proxy (testcontainers#3258)
  chore(deps): bump github/codeql-action from 3.29.3 to 3.30.3 (testcontainers#3287)
  chore(go): bump to Go 1.24 as minimal version (testcontainers#3298)
  deps(mongodb): update MongoDB Go Driver to v2 (testcontainers#3278)
  chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.5 to 4.25.6 (testcontainers#3224)
  chore(deps): bump mkdocs-include-markdown-plugin from 7.1.6 to 7.1.7 (testcontainers#3284)
  docs: clarify no client SDKs in production modules/images, in contributing.md (testcontainers#3279)
  chore(deps): bump github.com/docker/go-connections from 0.5.0 to 0.6.0 (testcontainers#3285)
  chore(deps): bump tj-actions/changed-files from 46.0.3 to 47.0.0 (testcontainers#3283)
  chore(modulegen): detect missing project files after new module creation (testcontainers#3281)
  chore(deps): bump github.com/docker/docker in /modules/nebulagraph (testcontainers#3277)
  feat(nebulagraph): add NebulaGraph module (testcontainers#3266)
  fix: preserve unix socket schema in testcontainers host from properties (testcontainers#3213)
  feat(registry): add helper functions to pull and tag images (testcontainers#3275)
  fix(reaper): remove termSignal override (testcontainers#3261)
  chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (testcontainers#3274)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug An issue with the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: goroutine leaks in reaper

2 participants