Merge pull request #46 from surban/master

Make CompoundFile implement Send + Sync

src/internal/entry.rs

@@ -1,8 +1,7 @@
 use crate::internal::{consts, DirEntry, MiniAllocator, ObjType, Timestamp};
-use std::cell::RefCell;
 use std::fmt;
 use std::path::{Path, PathBuf};
-use std::rc::Rc;
+use std::sync::{Arc, RwLock};
 use std::time::SystemTime;
 use uuid::Uuid;
 
@@ -125,14 +124,14 @@ pub struct Entries<'a, F: 'a> {
     // a reference to the Rc.  That would allow e.g. opening streams during
     // iteration.  But we'd need to think about how the iterator should behave
     // if the CFB tree structure is modified during iteration.
-    minialloc: &'a Rc<RefCell<MiniAllocator<F>>>,
+    minialloc: &'a Arc<RwLock<MiniAllocator<F>>>,
     stack: Vec<(PathBuf, u32, bool)>,
 }
 
 impl<'a, F> Entries<'a, F> {
     pub(crate) fn new(
         order: EntriesOrder,
-        minialloc: &'a Rc<RefCell<MiniAllocator<F>>>,
+        minialloc: &'a Arc<RwLock<MiniAllocator<F>>>,
         parent_path: PathBuf,
         start: u32,
     ) -> Entries<'a, F> {
@@ -149,7 +148,7 @@ impl<'a, F> Entries<'a, F> {
     }
 
     fn stack_left_spine(&mut self, parent_path: &Path, mut current_id: u32) {
-        let minialloc = self.minialloc.borrow();
+        let minialloc = self.minialloc.read().unwrap();
         while current_id != consts::NO_STREAM {
             self.stack.push((parent_path.to_path_buf(), current_id, true));
             current_id = minialloc.dir_entry(current_id).left_sibling;
@@ -162,7 +161,7 @@ impl<'a, F> Iterator for Entries<'a, F> {
 
     fn next(&mut self) -> Option<Entry> {
         if let Some((parent, stream_id, visit_siblings)) = self.stack.pop() {
-            let minialloc = self.minialloc.borrow();
+            let minialloc = self.minialloc.read().unwrap();
             let dir_entry = minialloc.dir_entry(stream_id);
             let path = join_path(&parent, dir_entry);
             if visit_siblings {
@@ -201,9 +200,8 @@ mod tests {
         Allocator, DirEntry, Directory, MiniAllocator, ObjType, Sectors,
         Timestamp, Validation, Version,
     };
-    use std::cell::RefCell;
     use std::path::{Path, PathBuf};
-    use std::rc::Rc;
+    use std::sync::{Arc, RwLock};
 
     fn make_entry(
         name: &str,
@@ -219,7 +217,7 @@ mod tests {
         dir_entry
     }
 
-    fn make_minialloc() -> Rc<RefCell<MiniAllocator<()>>> {
+    fn make_minialloc() -> Arc<RwLock<MiniAllocator<()>>> {
         // Root contains:      3 contains:
         //      5                  8
         //     / \                / \
@@ -261,7 +259,7 @@ mod tests {
             Validation::Strict,
         )
         .unwrap();
-        Rc::new(RefCell::new(minialloc))
+        Arc::new(RwLock::new(minialloc))
     }
 
     fn paths_for_entries(entries: &[Entry]) -> Vec<&Path> {

src/internal/stream.rs

@@ -1,7 +1,6 @@
 use crate::internal::{consts, MiniAllocator, ObjType, SectorInit};
-use std::cell::RefCell;
 use std::io::{self, BufRead, Read, Seek, SeekFrom, Write};
-use std::rc::{Rc, Weak};
+use std::sync::{Arc, RwLock, Weak};
 
 //===========================================================================//
 
@@ -11,7 +10,7 @@ const BUFFER_SIZE: usize = 8192;
 
 /// A stream entry in a compound file, much like a filesystem file.
 pub struct Stream<F> {
-    minialloc: Weak<RefCell<MiniAllocator<F>>>,
+    minialloc: Weak<RwLock<MiniAllocator<F>>>,
     stream_id: u32,
     total_len: u64,
     buffer: Box<[u8; BUFFER_SIZE]>,
@@ -23,12 +22,13 @@ pub struct Stream<F> {
 
 impl<F> Stream<F> {
     pub(crate) fn new(
-        minialloc: &Rc<RefCell<MiniAllocator<F>>>,
+        minialloc: &Arc<RwLock<MiniAllocator<F>>>,
         stream_id: u32,
     ) -> Stream<F> {
-        let total_len = minialloc.borrow().dir_entry(stream_id).stream_len;
+        let total_len =
+            minialloc.read().unwrap().dir_entry(stream_id).stream_len;
         Stream {
-            minialloc: Rc::downgrade(minialloc),
+            minialloc: Arc::downgrade(minialloc),
             stream_id,
             total_len,
             buffer: Box::new([0; BUFFER_SIZE]),
@@ -39,7 +39,7 @@ impl<F> Stream<F> {
         }
     }
 
-    fn minialloc(&self) -> io::Result<Rc<RefCell<MiniAllocator<F>>>> {
+    fn minialloc(&self) -> io::Result<Arc<RwLock<MiniAllocator<F>>>> {
         self.minialloc.upgrade().ok_or_else(|| {
             io::Error::new(io::ErrorKind::Other, "CompoundFile was dropped")
         })
@@ -83,7 +83,11 @@ impl<F: Read + Write + Seek> Stream<F> {
             let new_position = self.current_position().min(size);
             self.flush_changes()?;
             let minialloc = self.minialloc()?;
-            resize_stream(&mut minialloc.borrow_mut(), self.stream_id, size)?;
+            resize_stream(
+                &mut minialloc.write().unwrap(),
+                self.stream_id,
+                size,
+            )?;
             self.total_len = size;
             self.buf_offset_from_start = new_position;
             self.buf_pos = 0;
@@ -110,7 +114,7 @@ impl<F: Read + Seek> BufRead for Stream<F> {
             self.buf_pos = 0;
             let minialloc = self.minialloc()?;
             self.buf_cap = read_data_from_stream(
-                &mut minialloc.borrow_mut(),
+                &mut minialloc.write().unwrap(),
                 self.stream_id,
                 self.buf_offset_from_start,
                 &mut self.buffer[..],
@@ -234,7 +238,7 @@ impl<F: Read + Write + Seek> Write for Stream<F> {
     fn flush(&mut self) -> io::Result<()> {
         self.flush_changes()?;
         let minialloc = self.minialloc()?;
-        minialloc.borrow_mut().flush()?;
+        minialloc.write().unwrap().flush()?;
         Ok(())
     }
 }
@@ -257,13 +261,13 @@ impl<F: Read + Write + Seek> Flusher<F> for FlushBuffer {
     fn flush_changes(&self, stream: &mut Stream<F>) -> io::Result<()> {
         let minialloc = stream.minialloc()?;
         write_data_to_stream(
-            &mut minialloc.borrow_mut(),
+            &mut minialloc.write().unwrap(),
             stream.stream_id,
             stream.buf_offset_from_start,
             &stream.buffer[..stream.buf_cap],
         )?;
         debug_assert_eq!(
-            minialloc.borrow().dir_entry(stream.stream_id).stream_len,
+            minialloc.read().unwrap().dir_entry(stream.stream_id).stream_len,
             stream.total_len
         );
         Ok(())

src/lib.rs

@@ -45,13 +45,12 @@
 
 #![warn(missing_docs)]
 
-use std::cell::{Ref, RefCell, RefMut};
 use std::fmt;
 use std::fs;
 use std::io::{self, Read, Seek, SeekFrom, Write};
 use std::mem::size_of;
 use std::path::{Path, PathBuf};
-use std::rc::Rc;
+use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard};
 
 use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
 use fnv::FnvHashSet;
@@ -108,16 +107,16 @@ fn create_with_path(path: &Path) -> io::Result<CompoundFile<fs::File>> {
 /// [`File`](https://doc.rust-lang.org/std/fs/struct.File.html) or
 /// [`Cursor`](https://doc.rust-lang.org/std/io/struct.Cursor.html)).
 pub struct CompoundFile<F> {
-    minialloc: Rc<RefCell<MiniAllocator<F>>>,
+    minialloc: Arc<RwLock<MiniAllocator<F>>>,
 }
 
 impl<F> CompoundFile<F> {
-    fn minialloc(&self) -> Ref<MiniAllocator<F>> {
-        self.minialloc.borrow()
+    fn minialloc(&self) -> RwLockReadGuard<MiniAllocator<F>> {
+        self.minialloc.read().unwrap()
     }
 
-    fn minialloc_mut(&mut self) -> RefMut<MiniAllocator<F>> {
-        self.minialloc.borrow_mut()
+    fn minialloc_mut(&mut self) -> RwLockWriteGuard<MiniAllocator<F>> {
+        self.minialloc.write().unwrap()
     }
 
     /// Returns the CFB format version used for this compound file.
@@ -291,8 +290,8 @@ impl<F> CompoundFile<F> {
         // We only ever retain Weak copies of the CompoundFile's minialloc Rc
         // (e.g. in Stream structs), so the Rc::try_unwrap() should always
         // succeed.
-        match Rc::try_unwrap(self.minialloc) {
-            Ok(ref_cell) => ref_cell.into_inner().into_inner(),
+        match Arc::try_unwrap(self.minialloc) {
+            Ok(ref_cell) => ref_cell.into_inner().unwrap().into_inner(),
             Err(_) => unreachable!(),
         }
     }
@@ -557,7 +556,7 @@ impl<F: Read + Seek> CompoundFile<F> {
             validation,
         )?;
 
-        Ok(CompoundFile { minialloc: Rc::new(RefCell::new(minialloc)) })
+        Ok(CompoundFile { minialloc: Arc::new(RwLock::new(minialloc)) })
     }
 }
 
@@ -635,7 +634,7 @@ impl<F: Read + Write + Seek> CompoundFile<F> {
             consts::END_OF_CHAIN,
             Validation::Strict,
         )?;
-        Ok(CompoundFile { minialloc: Rc::new(RefCell::new(minialloc)) })
+        Ok(CompoundFile { minialloc: Arc::new(RwLock::new(minialloc)) })
     }
 
     /// Creates a new, empty storage object (i.e. "directory") at the provided

tests/basic.rs

@@ -769,3 +769,18 @@ fn drop_compound_file_with_stream_open() -> io::Result<()> {
 }
 
 //===========================================================================//
+// Tests for asserting Send + Sync:
+
+#[test]
+fn test_compound_file_send() {
+    fn assert_send<T: Send>() {}
+    assert_send::<CompoundFile<std::fs::File>>();
+}
+
+#[test]
+fn test_compound_file_sync() {
+    fn assert_sync<T: Sync>() {}
+    assert_sync::<CompoundFile<std::fs::File>>();
+}
+
+//===========================================================================//