Fixes memory leak due to sequences allocated by minimap2 not being freed

[charlesgregory]

Switches mm_idx from being a struct back to a pointer for Aligner 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. https://github.com/jguhlin/minimap2-rs/blob/b0023633ede58a176cb4adeea2f468938d97bb53/src/lib.rs#L769

Even if you were to free the original pointer with mm_idx_destroy after that line, it would later segfault since idx.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!

[jguhlin]

Awesome! It's my first ffi project so I knew there were memory leaks I've missed. I'm out of town so will get merged early Juy.

[jianshu93]

I can compile it but just found that the import is not necessary:

warning: unused import: std::ops::DerefMut --> src/lib.rs:51:5	51	use std::ops::DerefMut;	^^^^^^^^^^^^^^^^^^

= note: #[warn(unused_imports)] on by default

warning: minimap2 (lib) generated 1 warning (run cargo fix --lib -p minimap2 to apply 1 suggestion)

Other looks good.

Jianshu

[jianshu93]

hi @jguhlin, it would be nice if you can merge this to main and publish a new release on crates.io. I cannot wait to use the memory safe version.

Thanks!

Jianshu

[jguhlin]

@jianshu93 Hey, it's Sat here, but I'll get this on Monday. Been deep in grant writing and other work since I've been back. Cheers

[jguhlin]

@charlesgregory @jianshu93 Released in 0.1.19

[jianshu93]

Many thanks! this is so fast! Tests passed and I will rely on it for several softwares, e.g., ANI calculation, overlap detection.

Jianshu