-
Notifications
You must be signed in to change notification settings - Fork 437
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
Memory leak in yajl_tree_parse #168
base: master
Are you sure you want to change the base?
Conversation
d74c806
to
1b9509d
Compare
Rebased against master |
ctx.stack was not being cleaned up after a parse error. this patch pops all remaining elements and frees them.
I can confirm that this memory leak still exists in YAJL, and that this fixes it. It would be nice to see this merged. |
This has been open for 3 months now and it's an easy fix. What's the hold up? |
Checking in. This has been open for 9 months now. |
1 year and 4 months now. Does @lloyd still exist? |
There's been lots of forks of this lib and still no @lloyd maybe it's time to find a maintainer and create an "official" fork. |
Hey @lloyd, you there? |
Hi, Lloyd, is there any chances to have this PR merged? |
- use wrapped malloc() et al wrappers consistently - update example/parse_config.c to do memory leak detection - add a regression test using example/parse_config Several issues in lloyd/yajl complained about this leak, and comments in lloyd/yajl#102 showed a mostly correct fix though none of these issues mentioned or actually fixed the directly related error reporting problem. Fixes lloyd/yajl#102, fixes lloyd/yajl#113, fixes lloyd/yajl#168, fixes lloyd/yajl#191, fixes lloyd/yajl#223, fixes lloyd/yajl#250. Also fixes lloy/yajl#185.
A long time ago, I have done a ticket: |
The yajl_tree_parse function does not deallocate ctx.stack when yajl_complete_parse returns an error condition, resulting in a memory leak. The solution is to pop all elements off the stack and yajl_tree_free their values.