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

Fix memory leak #111

Merged
merged 5 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
### 0.1.23 minimap2 2.28
+ Fixed memory leak when not dropping mm_idx_t properly. This is done by adding in syntactic sugar in minimap2-sys @jguhlin

### 0.1.22 minimap2 2.28
#### Changes
+ Fixed a memory segfault when re-using a thread local buffer. Not sure why it occurs, but this fix seems to solve it.

### 0.1.21 minimap2 2.28
Contributors to this release: @mbhall88 @rob-p @Sam-Sims @charlesgregory @PB-DB
#### Breaking Changes
+ Map now returns Arc String's to reduce memory allocation for large and/or repetitive jobs
+ map now takes an additional argument, query_name: Option<&[u8]>, possibly solves [#75](https://github.com/jguhlin/minimap2-rs/issues/75) (@rob-p @mbhall88 @jguhlin)
+ Arc the Index, to prevent double-frees, solves [#71](https://github.com/jguhlin/minimap2-rs/issues/71)
+ Map file now passes in query name, which should help with [#75](https://github.com/jguhlin/minimap2-rs/issues/75)
+ Supplementary flag now better detected (@rob-p)
+ FIX: Cigar string missing softclip operation (@Sam-Sims)

#### Migration Guide
+ Make all setting changes before calling a with_index, with_seq's function
+ Change all map calls to include a query name, or None if not needed

### Other Changes
+ Add ergonomic functions n_seq and get_seq.
+ Better docs on applying presets, solves [#84](https://github.com/jguhlin/minimap2-rs/issues/84)
+ Better detection of target arch c_char's and ptr's, solves [#82](https://github.com/jguhlin/minimap2-rs/issues/82)
+ Support for M1 Mac compilation and addition of github workflows to test it, solving [#81](https://github.com/jguhlin/minimap2-rs/issues/81)
+ Rayon test, so some support, closes [#5](https://github.com/jguhlin/minimap2-rs/issues/5)
+ Static str's and now static CStr's
+ FIX: memory leak due to sequences allocated by minimap2 not being freed @charlesgregory
+ Add Send + Sync to Aligner, along with unit test @PB-DB
+ Experimental Android support (tested on aarch64 and x86_64), solves [#66](https://github.com/jguhlin/minimap2-rs/issues/66)
+ Added flag and option documents
+ Added with_gap_open penalty ergonomic function

### 0.1.20 minimap2 2.28
+ Fix htslib errors. No update to -sys crate needed.

### 0.1.19 minimap2 2.28
+ Fix memory leak by @charlesgregory

### 0.1.18 minimap2 2.28
+ Update to minimap2 v2.28 @jguhlin
+ Support for lrhqae preset @jguhlin

### 0.1.17 minimap2 2.27
* Mark bam::Record objects as supplementary. #52 @PB-DB
* Only use rust-htslib/curl when curl feature is enabled. #53 @PB-DB
* Update to minimap2 v2.27 @jguhlin
* Switch to needletail for reading fast files (features map-file) @jguhlin
* Convert functions to take slices of vectors instead of refs to vecs `&[Vec<u8>]` instead of `&Vec<Vec<u8>>` @jguhlin
* _breaking_ Curl is no longer a default option for htslib, please re-enable it as needed with cargo.toml features
* _breaking_ Now using needletail for map-files, enabled by default. However, compression algorithms are disabled. Please enable with cargo.toml features
* Experimental rayon support
* aligner.with_cigar_clipping() to add soft clipping to the CIGAR vec (with_cigar() still adds to only the string, following the minimap2 outputs for PAF)
* _breaking_ .with_threads(_) is now .with_index_threads(_) to make it more clear

### 0.1.16 minimap2 2.26
* Much better cross compilation support thanks to @Adoni5

### 0.1.15 minimap2 2.26
* Compilation on aarch64 thanks to @leiste375
* README corrections thanks to @wdecoster
* Better support for static builds / linking
* Update fffx to a version that uses bytelines without tokio. Drastically reduces compile times and dependency tree.

### 0.1.14 minimap2 2.26
* Memory leak fixed by @Adoni5
* Updated deps

### 0.1.13 minimap2 2.26
* Add with_seq to support indexing a single sequence (as per mappy: https://github.com/lh3/minimap2/blob/master/python/mappy.pyx#L115)
* minimap2-rs: update rust-htslib deps
* simdutf8 now optional dependency requiring map-file feature to be enabled
* Support soft-clipping string in CIGAR. WARNING: Does not support hard clipping. Please open an issue if you need this.
* Update minimap to 2.26
* Not convinced SSE41/SSE2 are working properly. Recommend simde.

### 0.1.11
* HTS lib: add support for optional quality scores by @eharr

### 0.1.10
* HTS lib support by @eharr
* HTS lib: Output sam/bam files by @eharr
* More tests by @eharr
* Display impl for Strand thanks to @ahcm
* Update minimap2-sys to latest version by @jguhlin
* -sys crate mm2fast added as additional backend by @jguhlin
* zlib dep changes by @jguhlin (hopefully now it is more portable and robust)
* -sys crate now supports SIMDe

## 0.1.9
* Thanks for @Adoni5 for switching to builder pattern, and @eharr for adding additional fields to alignment.
* Do not require libclang for normal compilation.
## 0.1.8
* Multithreading support (use less raw pointers, and treat more like rust Struct's)
## 0.1.7
* use libc instead of std:ffi::c_int as well
## 0.1.6
* Support slightly older versions of rustc by using libc:: rather than std::ffi for c_char (Thanks dwpeng!)
* Use fffx module for fasta/q parsing
10 changes: 5 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "minimap2"
version = "0.1.22+minimap2.2.28"
version = "0.1.23+minimap2.2.28"
edition = "2021"
authors = ["Joseph Guhlin <[email protected]>"]
license = "MIT OR Apache-2.0"
Expand Down Expand Up @@ -31,14 +31,14 @@ libc = "0.2"
needletail = { version = "0.6", optional = true, default-features = false}

# Dep for development
minimap2-sys = { path = "./minimap2-sys", version = "0.1.20+minimap2.2.28" }
minimap2-sys = { path = "./minimap2-sys" , version = "0.1.20+minimap2.2.28" }
# minimap2-sys = "0.1.19"
rust-htslib = { version = "0.48", default-features = false, optional = true }
rust-htslib = { version = "0.49", default-features = false, optional = true }

[dev-dependencies]
rayon = "1.10"
crossbeam = "0.8.4"
clap = { version = "4.5.21", features = ["derive"] }
clap = { version = "4.5.23", features = ["derive"] }
needletail = { version = "0.6", default-features = false}

# The end-user should decide this...
Expand All @@ -52,7 +52,7 @@ needletail = { version = "0.6", default-features = false}

[features]
default = ["map-file"]
map-file = ["needletail"] #, "simdutf8"]
map-file = ["needletail"]
htslib = ['rust-htslib']
simde = ["minimap2-sys/simde"]
zlib-ng = ["minimap2-sys/zlib-ng"]
Expand Down
99 changes: 2 additions & 97 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Tested with rustc 1.82.0 and nightly. So probably a good idea to upgrade before
## Minimap2 Version Table
| minimap2-rs | minimap2 |
|-------------|----------|
| 0.1.23 | 2.28 |
| 0.1.22 | 2.28 |
| 0.1.21 | 2.28 |
| 0.1.20 | 2.28 |
Expand Down Expand Up @@ -334,103 +335,7 @@ See [customization](#customization) for how to use these.
| `MM_I_NO_NAME` | `4` | Do not store sequence names in the index. |

# Changelog
### 0.1.22 minimap2 2.28
#### Changes
+ Fixed a memory segfault when re-using a thread local buffer. Not sure why it occurs, but this fix seems to solve it.

### 0.1.21 minimap2 2.28
Contributors to this release: @mbhall88 @rob-p @Sam-Sims @charlesgregory @PB-DB
#### Breaking Changes
+ Map now returns Arc String's to reduce memory allocation for large and/or repetitive jobs
+ map now takes an additional argument, query_name: Option<&[u8]>, possibly solves [#75](https://github.com/jguhlin/minimap2-rs/issues/75) (@rob-p @mbhall88 @jguhlin)
+ Arc the Index, to prevent double-frees, solves [#71](https://github.com/jguhlin/minimap2-rs/issues/71)
+ Map file now passes in query name, which should help with [#75](https://github.com/jguhlin/minimap2-rs/issues/75)
+ Supplementary flag now better detected (@rob-p)
+ FIX: Cigar string missing softclip operation (@Sam-Sims)

#### Migration Guide
+ Make all setting changes before calling a with_index, with_seq's function
+ Change all map calls to include a query name, or None if not needed

### Other Changes
+ Add ergonomic functions n_seq and get_seq.
+ Better docs on applying presets, solves [#84](https://github.com/jguhlin/minimap2-rs/issues/84)
+ Better detection of target arch c_char's and ptr's, solves [#82](https://github.com/jguhlin/minimap2-rs/issues/82)
+ Support for M1 Mac compilation and addition of github workflows to test it, solving [#81](https://github.com/jguhlin/minimap2-rs/issues/81)
+ Rayon test, so some support, closes [#5](https://github.com/jguhlin/minimap2-rs/issues/5)
+ Static str's and now static CStr's
+ FIX: memory leak due to sequences allocated by minimap2 not being freed @charlesgregory
+ Add Send + Sync to Aligner, along with unit test @PB-DB
+ Experimental Android support (tested on aarch64 and x86_64), solves [#66](https://github.com/jguhlin/minimap2-rs/issues/66)
+ Added flag and option documents
+ Added with_gap_open penalty ergonomic function

### 0.1.20 minimap2 2.28
+ Fix htslib errors. No update to -sys crate needed.

### 0.1.19 minimap2 2.28
+ Fix memory leak by @charlesgregory

### 0.1.18 minimap2 2.28
+ Update to minimap2 v2.28 @jguhlin
+ Support for lrhqae preset @jguhlin

### 0.1.17 minimap2 2.27
* Mark bam::Record objects as supplementary. #52 @PB-DB
* Only use rust-htslib/curl when curl feature is enabled. #53 @PB-DB
* Update to minimap2 v2.27 @jguhlin
* Switch to needletail for reading fast files (features map-file) @jguhlin
* Convert functions to take slices of vectors instead of refs to vecs `&[Vec<u8>]` instead of `&Vec<Vec<u8>>` @jguhlin
* _breaking_ Curl is no longer a default option for htslib, please re-enable it as needed with cargo.toml features
* _breaking_ Now using needletail for map-files, enabled by default. However, compression algorithms are disabled. Please enable with cargo.toml features
* Experimental rayon support
* aligner.with_cigar_clipping() to add soft clipping to the CIGAR vec (with_cigar() still adds to only the string, following the minimap2 outputs for PAF)
* _breaking_ .with_threads(_) is now .with_index_threads(_) to make it more clear

### 0.1.16 minimap2 2.26
* Much better cross compilation support thanks to @Adoni5

### 0.1.15 minimap2 2.26
* Compilation on aarch64 thanks to @leiste375
* README corrections thanks to @wdecoster
* Better support for static builds / linking
* Update fffx to a version that uses bytelines without tokio. Drastically reduces compile times and dependency tree.

### 0.1.14 minimap2 2.26
* Memory leak fixed by @Adoni5
* Updated deps

### 0.1.13 minimap2 2.26
* Add with_seq to support indexing a single sequence (as per mappy: https://github.com/lh3/minimap2/blob/master/python/mappy.pyx#L115)
* minimap2-rs: update rust-htslib deps
* simdutf8 now optional dependency requiring map-file feature to be enabled
* Support soft-clipping string in CIGAR. WARNING: Does not support hard clipping. Please open an issue if you need this.
* Update minimap to 2.26
* Not convinced SSE41/SSE2 are working properly. Recommend simde.

### 0.1.11
* HTS lib: add support for optional quality scores by @eharr

### 0.1.10
* HTS lib support by @eharr
* HTS lib: Output sam/bam files by @eharr
* More tests by @eharr
* Display impl for Strand thanks to @ahcm
* Update minimap2-sys to latest version by @jguhlin
* -sys crate mm2fast added as additional backend by @jguhlin
* zlib dep changes by @jguhlin (hopefully now it is more portable and robust)
* -sys crate now supports SIMDe

## 0.1.9
* Thanks for @Adoni5 for switching to builder pattern, and @eharr for adding additional fields to alignment.
* Do not require libclang for normal compilation.
## 0.1.8
* Multithreading support (use less raw pointers, and treat more like rust Struct's)
## 0.1.7
* use libc instead of std:ffi::c_int as well
## 0.1.6
* Support slightly older versions of rustc by using libc:: rather than std::ffi for c_char (Thanks dwpeng!)
* Use fffx module for fasta/q parsing
See [CHANGELOG.md](CHANGELOG.md)

# Funding
![Genomics Aotearoa](info/genomics-aotearoa.png)
2 changes: 1 addition & 1 deletion minimap2-sys/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "minimap2-sys"
version = "0.1.20+minimap2.2.28"
version = "0.1.21+minimap2.2.28"
edition = "2021"
links = "libminimap2"
authors = ["Joseph Guhlin <[email protected]>"]
Expand Down
3 changes: 3 additions & 0 deletions minimap2-sys/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ mm2-fast and minimap2 have diverged. At this point mm2-fast is no longer support
* Can we decouple from pthread? This would allow Windows and (possibly) WASM compilation.

## Changelog
### 0.1.21 minimap2.2.28
Syntactic sugar for mm_idx_t to support Drop, Deref, and DerefMut

### 0.1.20 minimap2.2.28
* Move Drop impl to -sys crate

Expand Down
31 changes: 30 additions & 1 deletion minimap2-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
#[cfg(all(not(feature = "bindgen")))]
include!("bindings.rs");

use std::{mem::MaybeUninit, ops::{DerefMut, Deref}};

unsafe impl Send for mm_idx_t {}
unsafe impl Send for mm_idx_reader_t {}
unsafe impl Send for mm_mapopt_t {}
Expand All @@ -18,8 +20,35 @@ impl Drop for mm_idx_t {
}
}

pub struct MmIdx {
pub idx: *mut mm_idx_t,
}

impl From<*mut mm_idx_t> for MmIdx {
fn from(idx: *mut mm_idx_t) -> Self {
MmIdx { idx }
}
}

use std::mem::MaybeUninit;
impl Drop for MmIdx {
fn drop(&mut self) {
unsafe { mm_idx_destroy(self.idx) };
}
}

impl Deref for MmIdx {
type Target = mm_idx_t;

fn deref(&self) -> &Self::Target {
unsafe { &*self.idx }
}
}

impl DerefMut for MmIdx {
fn deref_mut(&mut self) -> &mut Self::Target {
unsafe { &mut *self.idx }
}
}

impl Default for mm_mapopt_t {
fn default() -> Self {
Expand Down
6 changes: 2 additions & 4 deletions src/htslib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,8 @@ pub struct SeqMetaData {
pub is_alt: bool,
}

#[derive(Debug)]
pub struct MMIndex {
pub inner: Arc<*mut mm_ffi::mm_idx_t>,
pub inner: Arc<super::MmIdx>,
}

impl MMIndex {
Expand Down Expand Up @@ -345,8 +344,7 @@ impl MMIndex {
impl From<&Aligner<Built>> for MMIndex {
fn from(aligner: &Aligner<Built>) -> Self {
MMIndex {
// inner: aligner.idx.unwrap(),
inner: std::sync::Arc::clone(&aligner.idx.as_ref().unwrap()),
inner: std::sync::Arc::clone(aligner.idx.as_ref().unwrap()),
}
}
}
Expand Down
Loading
Loading