Fixes memory leak due to sequences allocated by minimap2 not being freed #59
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Switches
mm_idx
from being a struct back to a pointer forAligner
struct.When rust takes ownership here, it doesn't actually get ownership since it just does a
Copy
of the struct but the data allocated by minimap2 still exists.minimap2-rs/src/lib.rs
Line 769 in b002363
Even if you were to free the original pointer with
mm_idx_destroy
after that line, it would later segfault sinceidx.seq
is just a pointer to the sequences allocated by minimap2 here:https://github.com/lh3/minimap2/blob/69e36299168d739dded1c5549f662793af10da83/index.c#L425
So I changed the struct back to a pointer and used the manual
Drop
impl
to free the minimap2 index.Thanks for the crate btw!