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

[FIXED] Prevent memory leak on subscription failure #366

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

oncicaradupopovici
Copy link

  • modified subscribe fn to unregister sub from subMap when subscribe fails
  • mofiifed TestSubTimeout test

fixes #365

@oncicaradupopovici
Copy link
Author

Hello, I think there is a problem with your CI.
Uogradeing to at least Go 1.17 should fix it.

sub.go Outdated Show resolved Hide resolved
@kozlovic
Copy link
Member

Hello, I think there is a problem with your CI.

Yes, this project has not been updated for quite some time. I will update the Go versions. You may then want to rebase your PR when that is ready. I will ping here when that is done.

@kozlovic
Copy link
Member

@oncicaradupopovici I have updated main branch with updated dependencies and CI. If you could, please rebase so that the tests can run once you have updated your PR. Thanks!

@coveralls
Copy link

coveralls commented Jul 29, 2022

Coverage Status

Coverage increased (+0.09%) to 93.04% when pulling 3e2002b on oncicaradupopovici:main into cfc73f3 on nats-io:main.

@oncicaradupopovici
Copy link
Author

@kozlovic I have rebased and implemented the requested changes (strongly agree with those).
Thanks!

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for your contribution!

@kozlovic kozlovic changed the title unregister sub from subMap when subscribe fails [FIXED] Prevent memory leak on subscription failure Jul 29, 2022
@kozlovic kozlovic merged commit 64446ca into nats-io:main Jul 29, 2022
@oncicaradupopovici
Copy link
Author

Can we have a release, so I can close my issue on my side? Thank you!

@kozlovic
Copy link
Member

Ok, I will release v0.10.3.

@kozlovic
Copy link
Member

Done: https://github.com/nats-io/stan.go/releases/tag/v0.10.3

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

Successfully merging this pull request may close these issues.

Memory leak when subscribe fails
3 participants