Skip to content

Commit

Permalink
fix: memory leak in faidx when fetching sequences (#455)
Browse files Browse the repository at this point in the history
  • Loading branch information
brentp authored Dec 2, 2024
1 parent 7f3cfea commit d9fe03a
Showing 1 changed file with 16 additions and 14 deletions.
30 changes: 16 additions & 14 deletions src/faidx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,27 +85,27 @@ impl Reader {
/// * `name` - the name of the template sequence (e.g., "chr1")
/// * `begin` - the offset within the template sequence (starting with 0)
/// * `end` - the end position to return (if smaller than `begin`, the behavior is undefined).
pub fn fetch_seq<N: AsRef<str>>(&self, name: N, begin: usize, end: usize) -> Result<&[u8]> {
pub fn fetch_seq<N: AsRef<str>>(&self, name: N, begin: usize, end: usize) -> Result<Vec<u8>> {
if begin > std::i64::MAX as usize {

Check warning on line 89 in src/faidx/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

usage of a legacy numeric constant

warning: usage of a legacy numeric constant --> src/faidx/mod.rs:89:20 | 89 | if begin > std::i64::MAX as usize { | ^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants help: use the associated constant instead | 89 | if begin > i64::MAX as usize { | ~~~~~~~~
return Err(Error::FaidxPositionTooLarge);
}
if end > std::i64::MAX as usize {

Check warning on line 92 in src/faidx/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

usage of a legacy numeric constant

warning: usage of a legacy numeric constant --> src/faidx/mod.rs:92:18 | 92 | if end > std::i64::MAX as usize { | ^^^^^^^^^^^^^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#legacy_numeric_constants help: use the associated constant instead | 92 | if end > i64::MAX as usize { | ~~~~~~~~
return Err(Error::FaidxPositionTooLarge);
}
let cname = ffi::CString::new(name.as_ref().as_bytes()).unwrap();
let len_out: i64 = 0;
let cseq = unsafe {
let ptr = htslib::faidx_fetch_seq64(
self.inner, //*const faidx_t,
cname.as_ptr(), // c_name
begin as htslib::hts_pos_t, // p_beg_i
end as htslib::hts_pos_t, // p_end_i
&mut (len_out as htslib::hts_pos_t), //len
);
ffi::CStr::from_ptr(ptr)
let mut len_out: htslib::hts_pos_t = 0;
let ptr = unsafe {
htslib::faidx_fetch_seq64(
self.inner, //*const faidx_t,
cname.as_ptr(), // c_name
begin as htslib::hts_pos_t, // p_beg_i
end as htslib::hts_pos_t, // p_end_i
&mut len_out, //len
)
};

Ok(cseq.to_bytes())
let vec =
unsafe { Vec::from_raw_parts(ptr as *mut u8, len_out as usize, len_out as usize) };
Ok(vec)
}

/// Fetches the sequence and returns it as string.
Expand All @@ -122,7 +122,7 @@ impl Reader {
end: usize,
) -> Result<String> {
let bytes = self.fetch_seq(name, begin, end)?;
Ok(std::str::from_utf8(bytes).unwrap().to_owned())
Ok(std::str::from_utf8(&bytes).unwrap().to_owned())
}

/// Fetches the number of sequences in the fai index
Expand Down Expand Up @@ -226,13 +226,15 @@ mod tests {
fn faidx_read_chr_start() {
let r = open_reader();

//for _i in 0..100_000_000 { // loop to check for memory leaks
let bseq = r.fetch_seq("chr1", 0, 9).unwrap();
assert_eq!(bseq.len(), 10);
assert_eq!(bseq, b"GGGCACAGCC");

let seq = r.fetch_seq_string("chr1", 0, 9).unwrap();
assert_eq!(seq.len(), 10);
assert_eq!(seq, "GGGCACAGCC");
//}
}

#[test]
Expand Down

0 comments on commit d9fe03a

Please sign in to comment.