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.

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

[receiver/jmx] Fix memory leak on shutdown #32289

Closed
wants to merge 2 commits into from

Conversation

crobert-1
Copy link
Member

Description:

The receiver was starting a goroutine that would run without being stopped during shutdown. This changes the goroutine to be stopped during shutdown.

goleak is also added as a part of this change.

Link to tracking Issue:
#30438

Testing:
All existing tests are passing, as well as added goleak check.

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@andrzej-stencel
Copy link
Member

andrzej-stencel commented Apr 10, 2024

Was it a GitHub hiccup? I merged this PR and can see the merge commit: 36084a9, but this PR was still showing as open. I closed the PR, but it's actually merged. 😵

mx-psi pushed a commit to dineshg13/opentelemetry-collector-contrib that referenced this pull request Apr 10, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The receiver was starting a goroutine that would run without being
stopped during shutdown. This changes the goroutine to be stopped during
shutdown.

`goleak` is also added as a part of this change.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing, as well as added `goleak` check.
@crobert-1
Copy link
Member Author

Not sure what happened, but it's in main so it works for me 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
receiver/jmx JMX Receiver
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants