-
Notifications
You must be signed in to change notification settings - Fork 8
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
[#389] Fix KeyVal memory leak #388
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find.
Do the base tests pass?
Let's put that ASan output in the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I feel like... OP is good enough to copy into an issue to link and this should be good, assuming tests are passing.
Edit: Oops, jinx... very delayed jinx...
Forgot to say, this fix was found & tested to be good using the core/base tests. |
Good. Let's get an issue attached to the commit and we can get this in. |
Issue created. |
Go ahead and add the issue number, no pound. |
4fecc4e
to
0988081
Compare
Added the issue number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pound it.
0988081
to
8de3a47
Compare
Pounded. |
Found using ASAN, logs follow: