fix: Reason about Send/Sync-ness of types and change Rcs to Arcs

src/bam/buffer.rs

@@ -157,12 +157,12 @@ impl RecordBuffer {
     }
 
     /// Iterate over records that have been fetched with `fetch`.
-    pub fn iter(&self) -> vec_deque::Iter<Rc<bam::Record>> {
+    pub fn iter(&'_ self) -> vec_deque::Iter<'_, Rc<bam::Record>> {
         self.inner.iter()
     }
 
     /// Iterate over mutable references to records that have been fetched with `fetch`.
-    pub fn iter_mut(&mut self) -> vec_deque::IterMut<Rc<bam::Record>> {
+    pub fn iter_mut(&'_ mut self) -> vec_deque::IterMut<'_, Rc<bam::Record>> {
         self.inner.iter_mut()
     }
 

src/bam/header.rs

@@ -106,7 +106,7 @@ impl Header {
     }
 
     /// Returns an iterator of comment lines.
-    pub fn comments(&self) -> impl Iterator<Item = Cow<str>> {
+    pub fn comments(&'_ self) -> impl Iterator<Item = Cow<'_, str>> {
         self.records.iter().flat_map(|r| {
             r.split(|x| x == &b'\n')
                 .filter(|x| x.starts_with(b"@CO\t"))

src/bam/mod.rs

@@ -21,6 +21,7 @@ use std::path::Path;
 use std::rc::Rc;
 use std::slice;
 use std::str;
+use std::sync::Arc;
 
 use url::Url;
 
@@ -250,7 +251,7 @@ pub trait Read: Sized {
 #[derive(Debug)]
 pub struct Reader {
     htsfile: *mut htslib::htsFile,
-    header: Rc<HeaderView>,
+    header: Arc<HeaderView>,
     tpool: Option<ThreadPool>,
 }
 
@@ -298,7 +299,7 @@ impl Reader {
 
         Ok(Reader {
             htsfile,
-            header: Rc::new(HeaderView::new(header)),
+            header: Arc::new(HeaderView::new(header)),
             tpool: None,
         })
     }
@@ -383,7 +384,7 @@ impl Read for Reader {
             -2 => Some(Err(Error::BamTruncatedRecord)),
             -4 => Some(Err(Error::BamInvalidRecord)),
             _ => {
-                record.set_header(Rc::clone(&self.header));
+                record.set_header(Arc::clone(&self.header));
 
                 Some(Ok(()))
             }
@@ -591,8 +592,8 @@ impl<'a, T: AsRef<[u8]>, X: Into<FetchCoordinate>, Y: Into<FetchCoordinate>> Fro
 #[derive(Debug)]
 pub struct IndexedReader {
     htsfile: *mut htslib::htsFile,
-    header: Rc<HeaderView>,
-    idx: Rc<IndexView>,
+    header: Arc<HeaderView>,
+    idx: Arc<IndexView>,
     itr: Option<*mut htslib::hts_itr_t>,
     tpool: Option<ThreadPool>,
 }
@@ -637,8 +638,8 @@ impl IndexedReader {
         } else {
             Ok(IndexedReader {
                 htsfile,
-                header: Rc::new(HeaderView::new(header)),
-                idx: Rc::new(IndexView::new(idx)),
+                header: Arc::new(HeaderView::new(header)),
+                idx: Arc::new(IndexView::new(idx)),
                 itr: None,
                 tpool: None,
             })
@@ -665,8 +666,8 @@ impl IndexedReader {
         } else {
             Ok(IndexedReader {
                 htsfile,
-                header: Rc::new(HeaderView::new(header)),
-                idx: Rc::new(IndexView::new(idx)),
+                header: Arc::new(HeaderView::new(header)),
+                idx: Arc::new(IndexView::new(idx)),
                 itr: None,
                 tpool: None,
             })
@@ -908,15 +909,14 @@ impl IndexedReader {
 #[derive(Debug)]
 pub struct IndexView {
     inner: *mut hts_sys::hts_idx_t,
-    owned: bool,
 }
 
+unsafe impl Send for IndexView {}
+unsafe impl Sync for IndexView {}
+
 impl IndexView {
     fn new(hts_idx: *mut hts_sys::hts_idx_t) -> Self {
-        Self {
-            inner: hts_idx,
-            owned: true,
-        }
+        Self { inner: hts_idx }
     }
 
     #[inline]
@@ -960,10 +960,8 @@ impl IndexView {
 
 impl Drop for IndexView {
     fn drop(&mut self) {
-        if self.owned {
-            unsafe {
-                htslib::hts_idx_destroy(self.inner);
-            }
+        unsafe {
+            htslib::hts_idx_destroy(self.inner);
         }
     }
 }
@@ -977,7 +975,7 @@ impl Read for IndexedReader {
                     -2 => Some(Err(Error::BamTruncatedRecord)),
                     -4 => Some(Err(Error::BamInvalidRecord)),
                     _ => {
-                        record.set_header(Rc::clone(&self.header));
+                        record.set_header(Arc::clone(&self.header));
 
                         Some(Ok(()))
                     }
@@ -1060,7 +1058,7 @@ impl Format {
 #[derive(Debug)]
 pub struct Writer {
     f: *mut htslib::htsFile,
-    header: Rc<HeaderView>,
+    header: Arc<HeaderView>,
     tpool: Option<ThreadPool>,
 }
 
@@ -1134,7 +1132,7 @@ impl Writer {
 
         Ok(Writer {
             f,
-            header: Rc::new(HeaderView::new(header_record)),
+            header: Arc::new(HeaderView::new(header_record)),
             tpool: None,
         })
     }
@@ -1364,9 +1362,11 @@ fn itr_next(
 #[derive(Debug)]
 pub struct HeaderView {
     inner: *mut htslib::bam_hdr_t,
-    owned: bool,
 }
 
+unsafe impl Send for HeaderView {}
+unsafe impl Sync for HeaderView {}
+
 impl HeaderView {
     /// Create a new HeaderView from a pre-populated Header object
     pub fn from_header(header: &Header) -> Self {
@@ -1399,8 +1399,8 @@ impl HeaderView {
     }
 
     /// Create a new HeaderView from the underlying Htslib type, and own it.
-    pub fn new(inner: *mut htslib::bam_hdr_t) -> Self {
-        HeaderView { inner, owned: true }
+    fn new(inner: *mut htslib::bam_hdr_t) -> Self {
+        HeaderView { inner }
     }
 
     #[inline]
@@ -1480,17 +1480,14 @@ impl Clone for HeaderView {
     fn clone(&self) -> Self {
         HeaderView {
             inner: unsafe { htslib::sam_hdr_dup(self.inner) },
-            owned: true,
         }
     }
 }
 
 impl Drop for HeaderView {
     fn drop(&mut self) {
-        if self.owned {
-            unsafe {
-                htslib::sam_hdr_destroy(self.inner);
-            }
+        unsafe {
+            htslib::sam_hdr_destroy(self.inner);
         }
     }
 }

src/bam/record.rs

@@ -11,9 +11,9 @@ use std::marker::PhantomData;
 use std::mem::{size_of, MaybeUninit};
 use std::ops;
 use std::os::raw::c_char;
-use std::rc::Rc;
 use std::slice;
 use std::str;
+use std::sync::Arc;
 
 use byteorder::{LittleEndian, ReadBytesExt};
 
@@ -53,7 +53,7 @@ pub struct Record {
     pub inner: htslib::bam1_t,
     own: bool,
     cigar: Option<CigarStringView>,
-    header: Option<Rc<HeaderView>>,
+    header: Option<Arc<HeaderView>>,
 }
 
 unsafe impl Send for Record {}
@@ -183,7 +183,7 @@ impl Record {
         }
     }
 
-    pub fn set_header(&mut self, header: Rc<HeaderView>) {
+    pub fn set_header(&mut self, header: Arc<HeaderView>) {
         self.header = Some(header);
     }
 
@@ -833,7 +833,7 @@ impl Record {
     ///
     /// When an error occurs, the `Err` variant will be returned
     /// and the iterator will not be able to advance anymore.
-    pub fn aux_iter(&self) -> AuxIter {
+    pub fn aux_iter(&'_ self) -> AuxIter<'_> {
         AuxIter {
             // In order to get to the aux data section of a `bam::Record`
             // we need to skip fields in front of it
@@ -1118,14 +1118,14 @@ impl Record {
     ///    }
     ///    assert_eq!(mod_count, 14);
     /// ```
-    pub fn basemods_iter(&self) -> Result<BaseModificationsIter> {
+    pub fn basemods_iter(&'_ self) -> Result<BaseModificationsIter<'_>> {
         BaseModificationsIter::new(self)
     }
 
     /// An iterator that returns all of the modifications for each position as a vector.
     /// This is useful for the case where multiple possible modifications can be annotated
     /// at a single position (for example a C could be 5-mC or 5-hmC)
-    pub fn basemods_position_iter(&self) -> Result<BaseModificationsPositionIter> {
+    pub fn basemods_position_iter(&'_ self) -> Result<BaseModificationsPositionIter<'_>> {
         BaseModificationsPositionIter::new(self)
     }
 
@@ -1504,7 +1504,7 @@ where
     }
 
     /// Returns an iterator over the array.
-    pub fn iter(&self) -> AuxArrayIter<T> {
+    pub fn iter(&'_ self) -> AuxArrayIter<'_, T> {
         AuxArrayIter {
             index: 0,
             array: self,

src/bcf/header.rs

@@ -34,9 +34,9 @@
 
 use std::ffi;
 use std::os::raw::c_char;
-use std::rc::Rc;
 use std::slice;
 use std::str;
+use std::sync::Arc;
 
 use crate::htslib;
 
@@ -65,10 +65,13 @@ custom_derive! {
 /// A BCF header.
 #[derive(Debug)]
 pub struct Header {
-    pub inner: *mut htslib::bcf_hdr_t,
+    pub(crate) inner: *mut htslib::bcf_hdr_t,
     pub subset: Option<SampleSubset>,
 }
 
+unsafe impl Send for Header {}
+unsafe impl Sync for Header {}
+
 impl Default for Header {
     fn default() -> Self {
         Self::new()
@@ -266,11 +269,14 @@ pub enum HeaderRecord {
 
 #[derive(Debug)]
 pub struct HeaderView {
-    pub inner: *mut htslib::bcf_hdr_t,
+    pub(crate) inner: *mut htslib::bcf_hdr_t,
 }
 
+unsafe impl Send for HeaderView {}
+unsafe impl Sync for HeaderView {}
+
 impl HeaderView {
-    pub fn new(inner: *mut htslib::bcf_hdr_t) -> Self {
+    pub(crate) fn new(inner: *mut htslib::bcf_hdr_t) -> Self {
         HeaderView { inner }
     }
 
@@ -513,8 +519,15 @@ impl HeaderView {
     /// Create an empty record using this header view.
     ///
     /// The record can be reused multiple times.
+    ///
+    /// # Performance
+    ///
+    /// This is quite slow and resource-intensive as it clones the actual
+    /// header, instead of the reference to it. Therefore, whenever possible
+    /// / feasible you should use [`Record::new`](crate::bcf::Record::new) with
+    /// a reference to your header.
     pub fn empty_record(&self) -> crate::bcf::Record {
-        crate::bcf::Record::new(Rc::new(self.clone()))
+        crate::bcf::Record::new(Arc::new(self.clone()))
     }
 }