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

Memory leaks #29

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Memory leaks #29

merged 3 commits into from
Dec 3, 2021

Conversation

jezwhite
Copy link

Fix for #27 and a few other minor memory leaks

@jezwhite
Copy link
Author

jezwhite commented Oct 9, 2021

Ok, this PR has issues.

I've been running this branch in a production environment for a the last week and the leak fixes have made a huge difference on memory consumption. However, I have had a couple of random crashes. The crashes are related to #28. What is happening is that depending on what order the containers are destroyed, the SV's for output/errors no longer exist which then causes the crash. In the base branch, the SV's for output/errors are never freed, so no crash. I will work on a fix.

@gonzus
Copy link
Owner

gonzus commented Oct 11, 2021

Hey, thanks for the PR and comments. I was away for a while, but I will be happy to review and merge once you declare the fixes done. Cheers!

@jezwhite
Copy link
Author

Ok, added another commit, with an additional test. Straightforward enough change, but you may have issues with the way I stored the Duk pointer into the ctx instance, but I think it's the 'best' way?

/* save our duk pointer to the ctx so it can get it back later when we do callbacks */
duk_push_thread_stash(duk->ctx,duk->ctx);
duk_push_pointer(duk->ctx, duk);
duk_put_prop_string(duk->ctx, -2, PL_NAME_CONSOLE_GENERIC_CALLBACK); 
duk_pop(duk->ctx);

/* Get our duk pointer for this ctx*/
duk_push_thread_stash(ctx,ctx);
duk_get_prop_string(ctx, -1, PL_NAME_CONSOLE_GENERIC_CALLBACK); 
ourData = duk_get_pointer(ctx, -1);
duk_pop(ctx);

#define PL_NAME_CONSOLE_GENERIC_CALLBACK  "__perl__duk__"

Any questions, comments, let me know.

@gonzus gonzus merged commit 4466828 into gonzus:master Dec 3, 2021
@gonzus
Copy link
Owner

gonzus commented Dec 3, 2021

Merged more or less blindly -- let me know if anything broke. Thanks!

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.

2 participants