zesterer / chumsky

Write expressive, high-performance parsers with ease.
https://crates.io/crates/chumsky
MIT License
3.64k stars 155 forks source link

Add recursive_fold and recursive_2 to prevent memory leaks #664

Open jkylling opened 2 months ago

jkylling commented 2 months ago

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 https://github.com/zesterer/chumsky/issues/486#issuecomment-2198514803

If code is migrated to use recursive_2 or recursive_fold we should be able to resolve https://github.com/zesterer/chumsky/issues/486 I have tested that this patch for 0.9 fixes the memory issue on jaq when repeatedly creating parsers.

zesterer commented 2 months ago

How do you think this solution compares to #494 ?

jkylling commented 2 months ago

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.