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

Add recursive_fold and recursive_2 to prevent memory leaks #664

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkylling
Copy link

By using recursive_2 or recursive_fold instead of Recursive::declare we can prevent situations where we either keep too many strong references around, leading to memory leaks, or too few strong references around, leading to broken parsers because all the strong references and the parsers are dropped.

We could consider removing Recursive::declare from the public API, since it is hard to use it correctly.

Please see the investigation (for version 0.9) in #486 (comment)

If code is migrated to use recursive_2 or recursive_fold we should be able to resolve #486 I have tested that this patch for 0.9 fixes the memory issue on jaq when repeatedly creating parsers.

@zesterer
Copy link
Owner

zesterer commented Sep 1, 2024

How do you think this solution compares to #494 ?

@jkylling
Copy link
Author

jkylling commented Sep 2, 2024

How do you think this solution compares to #494 ?

I have not looked at #494 in detail, but I believe it allows accidentally dropping one of the parsers while keeping the other one. That would render the remaining parser unusable. The API should try to prevent accidental misuse if possible.

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 in recursive when using define function
2 participants