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

Fixing a memory leak when another broker connects. Closes #2057 #2064

Closed
wants to merge 1 commit into from

Conversation

przemyslawzygmunt
Copy link
Contributor

Due to unfamiliarity with your code, I am not sure if using context__cleanup in the place I suggested is correct, but in my tests it solves the problem.

@ralight
Copy link
Contributor

ralight commented Feb 2, 2021

I'm afraid it isn't correct, it results in a use-after-free in some situations. Thanks very much for spending time looking at it though. Leave it with me and I'll have a fix in place soon.

@ralight ralight closed this Feb 2, 2021
@przemyslawzygmunt
Copy link
Contributor Author

I was expecting that this is not the right place. As I mentioned, I spent several dozen minutes reviewing the code, which is not enough to properly define where to call context__cleanup.

@ralight
Copy link
Contributor

ralight commented Feb 2, 2021

Yes, it took me most of last evening to verify that it wasn't correct :)

@przemyslawzygmunt
Copy link
Contributor Author

However, I hope you managed to reproduce the leak described in #2057.

@ralight
Copy link
Contributor

ralight commented Feb 2, 2021

Yes I did, thank you for your help there.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants