Clean-up and thread safety

4 files changed, 199 insertions, 167 deletions

diff --git a/Cargo.toml b/Cargo.toml
index d01eb57..3aca997 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,7 +10,8 @@ bindgen = "0.53.1"
 [dependencies]
 lazy_static = "1.4"
 libc = "0.2"
-mmap = "0.1"
+memmap = "0.7"
+num_cpus = "1.13"
 num-traits = "0.2"
 num-derive = "0.3"
 owning_ref = "0.4"
diff --git a/src/lib.rs b/src/lib.rs
index 5769b54..5c737d4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,6 +1,6 @@
 #[macro_use] extern crate lazy_static;
 extern crate libc;
-extern crate mmap;
+extern crate memmap;
 extern crate num_derive;
 extern crate num_traits;
 extern crate owning_ref;
@@ -62,18 +62,21 @@ pub enum SquashfsError {
 	#[error("{0}")] LibraryNullError(String),
 	#[error("Symbolic link chain exceeds {0} elements")] LinkChain(i32), // Can I use a const in the formatting string?
 	#[error("Symbolic link loop detected containing {0}")] LinkLoop(PathBuf),
+	#[error("Dangling symbolic link from {0} to {1}")] DanglingLink(PathBuf, PathBuf),
 	#[error("{0} is type {1}, not {2}")] WrongType(String, String, String),
 	#[error("Tried to copy an object that can't be copied")] Copy,
 	#[error("Tried to get parent of a node with an unknown path")] NoPath,
 	#[error("Inode index {0} is not within limits 1..{1}")] Range(u64, u64),
 	#[error("Couldn't read file: {0}")] Read(#[from] std::io::Error),
 	#[error("The filesystem does not support the feature: {0}")] Unsupported(String),
-	#[error("Memory mapping failed: {0}")] Mmap(#[from] mmap::MapError),
+	#[error("Memory mapping failed: {0}")] Mmap(std::io::Error),
 	#[error("Couldn't get the current system time: {0}")] Time(#[from] std::time::SystemTimeError),
 	#[error("Refusing to create empty archive")] Empty,
 	#[error("Tried to write directory {0} before child {1}")] WriteOrder(u32, u32),
 	#[error("Tried to write unknown or unsupported file type")] WriteType(std::fs::FileType),
 	#[error("Callback returned an error")] WrappedError(BoxedError),
+	#[error("Failed to retrieve xattrs for {0}: {1}")] Xattr(PathBuf, std::io::Error),
+	#[error("Tried to add files to a writer that was already finished")] Finished,
 }
 
 type Result<T> = std::result::Result<T, SquashfsError>;
diff --git a/src/read.rs b/src/read.rs
index 7352b36..e708f12 100644
--- a/src/read.rs
+++ b/src/read.rs
@@ -1,22 +1,19 @@
 use std::collections::{HashMap, HashSet};
-use std::ffi::{CStr, CString, OsStr, OsString};
+use std::ffi::{CStr, CString};
 use std::io;
 use std::io::{Read, Seek};
 use std::path::{Path, PathBuf, Component};
-use std::os::unix::io::AsRawFd; // TODO Is there a way to mmap cross-platform?
 use std::sync::{Arc, Mutex};
-use bindings::*;
 use super::*;
-use mmap::{MemoryMap, MapOption};
+use memmap::{Mmap, MmapOptions};
 use owning_ref::OwningHandle;
-use thiserror::Error;
 
 // Canonicalize without requiring the path to actually exist in the filesystem
 fn dumb_canonicalize(path: &Path) -> PathBuf {
 	let mut ret = PathBuf::new();
 	for part in path.components() {
 		match part {
-			Component::Prefix(_) => panic!("What is this, Windows?"), // TODO
+			Component::Prefix(_) => (),
 			Component::CurDir => (),
 			Component::RootDir => ret.clear(),
 			Component::ParentDir => { ret.pop(); },
@@ -56,38 +53,41 @@ impl<'a> Dir<'a> {
 		unsafe { sqfs_dir_reader_rewind(**self.reader.lock().expect(LOCK_ERR)); }
 	}
 
-	fn read<'b>(&'b self) -> Result<Node<'a>> {
+	fn read<'b>(&'b self) -> Result<Option<Node<'a>>> {
 		let locked_reader = self.reader.lock().expect(LOCK_ERR);
-		let entry = sfs_init_ptr(&|x| unsafe {
-			sqfs_dir_reader_read(**locked_reader, x)
-		}, "Couldn't read directory entries", libc_free)?;
-		let name_bytes = unsafe { (**entry).name.as_slice((**entry).size as usize + 1) };
-		let name = String::from_utf8(name_bytes.to_vec())?;
-		let node = sfs_init_ptr(&|x| unsafe {
-			sqfs_dir_reader_get_inode(**locked_reader, x)
-		}, "Couldn't read directory entry inode", libc_free)?;
-		Node::new(self.node.container, node, self.node.path.as_ref().map(|path| path.join(name)))
+		let mut raw_entry: *mut sqfs_dir_entry_t = ptr::null_mut();
+		if sfs_check(unsafe { sqfs_dir_reader_read(**locked_reader, &mut raw_entry) }, "Couldn't read directory entries")? > 0 { Ok(None) }
+		else if raw_entry.is_null() { Err(SquashfsError::LibraryReturnError("Couldn't read directory entries".to_string()))? }
+		else {
+			let entry = ManagedPointer::new(raw_entry, libc_free);
+			let name_bytes = unsafe { (**entry).name.as_slice((**entry).size as usize + 1) };
+			let name = String::from_utf8(name_bytes.to_vec())?;
+			let node = sfs_init_ptr(&|x| unsafe {
+				sqfs_dir_reader_get_inode(**locked_reader, x)
+			}, "Couldn't read directory entry inode", libc_free)?;
+			Ok(Some(Node::new(self.node.container, node, self.node.path.as_ref().map(|path| path.join(name)))?))
+		}
 	}
 
 	pub fn child(&self, name: &str) -> Result<Option<Node>> {
 		match unsafe { enoent_ok(sfs_check(sqfs_dir_reader_find(**self.reader.lock().expect(LOCK_ERR), CString::new(name)?.as_ptr()), &format!("Couldn't find child \"{}\"", name)))? } {
 			None => Ok(None),
-			Some(_) => Ok(Some(self.read()?)),
+			Some(_) => Ok(self.read()?),
 		}
 	}
 }
 
 impl<'a> std::iter::Iterator for Dir<'a> {
-	type Item = Node<'a>;
+	type Item = Result<Node<'a>>;
 
 	fn next(&mut self) -> Option<Self::Item> {
-		self.read().ok()
+		self.read().transpose()
 	}
 }
 
 pub struct File<'a> {
 	node: &'a Node<'a>,
-	compressor: ManagedPointer<sqfs_compressor_t>,
+	#[allow(dead_code)] compressor: ManagedPointer<sqfs_compressor_t>, // Referenced by `reader`
 	reader: Mutex<ManagedPointer<sqfs_data_reader_t>>,
 	offset: Mutex<u64>,
 }
@@ -129,7 +129,7 @@ impl<'a> File<'a> {
 				_ => panic!("File is not a file")
 			}
 		};
-		let block_count = unsafe { inode.payload_bytes_used / std::mem::size_of::<sqfs_u32>() as u32 };
+		let block_count = inode.payload_bytes_used / std::mem::size_of::<sqfs_u32>() as u32;
 		if block_count == 0 || frag_idx != 0xffffffff { return None; }
 		let block_sizes = unsafe { inode.extra.as_slice(block_count as usize) };
 		if block_sizes.iter().any(|x| x & 0x01000000 == 0) { return None; }
@@ -225,7 +225,7 @@ impl<'a> Data<'a> {
 		}
 	}
 	
-	fn name(&self) -> String {
+	pub fn name(&self) -> String {
 		match self {
 			Data::File(_) => "regular file",
 			Data::Dir(_) => "directory",
@@ -281,7 +281,7 @@ pub struct OwnedDir<'a> {
 }
 
 impl<'a> std::iter::Iterator for OwnedDir<'a> {
-	type Item = Node<'a>;
+	type Item = Result<Node<'a>>;
 
 	fn next(&mut self) -> Option<Self::Item> {
 		(*self.handle).next()
@@ -315,6 +315,11 @@ impl<'a> Node<'a> {
 
 	pub fn xattrs(&self, category: XattrType) -> Result<HashMap<Vec<u8>, Vec<u8>>> {
 		if self.container.superblock.flags & SQFS_SUPER_FLAGS_SQFS_FLAG_NO_XATTRS as u16 != 0 { Ok(HashMap::new()) }
+		// TODO The following line reflects what I think is a bug.  I have a non-xattr archive
+		// created by mksquashfs, which does not have the above flag set but has the below table
+		// offset of -1.  This workaround allows us to check both cases until I get around to
+		// figuring out what's going on.
+		else if self.container.superblock.xattr_id_table_start == 0xffffffffffffffff { Ok(HashMap::new()) }
 		else {
 			let compressor = self.container.compressor()?;
 			let xattr_reader = sfs_init_check_null(&|| unsafe {
@@ -378,7 +383,7 @@ impl<'a> Node<'a> {
 		}).ok_or(SquashfsError::NoPath)?
 	}
 
-	pub fn resolve(&self) -> Result<Self> {
+	pub fn resolve_exists(&self) -> Result<Self> {
 		let mut visited = HashSet::new();
 		let mut cur = Box::new(self.clone());
 		let mut i = 0;
@@ -405,18 +410,36 @@ impl<'a> Node<'a> {
 		}
 	}
 
+	pub fn resolve(&self) -> Result<Option<Self>> {
+		enoent_ok(self.resolve_exists())
+	}
+
+	pub fn is_file(&self) -> Result<bool> {
+		match self.data()? {
+			Data::File(_) => Ok(true),
+			_ => Ok(false),
+		}
+	}
+
 	pub fn as_file(&self) -> Result<File> {
 		match self.data()? {
 			Data::File(f) => Ok(f),
 			other => Err(SquashfsError::WrongType(self.path_string(), other.name(), "regular file".to_string())),
 		}
 	}
-
+	
 	pub fn into_owned_file(self) -> Result<OwnedFile<'a>> {
-		let resolved = self.resolve()?;
+		let resolved = self.resolve_exists()?;
 		Ok(OwnedFile { handle: OwningHandle::try_new(Box::new(resolved), |x| unsafe { (*x).as_file().map(|x| Box::new(x)) })? })
 	}
 
+	pub fn is_dir(&self) -> Result<bool> {
+		match self.data()? {
+			Data::Dir(_) => Ok(true),
+			_ => Ok(false),
+		}
+	}
+
 	pub fn as_dir(&self) -> Result<Dir> {
 		match self.data()? {
 			Data::Dir(d) => Ok(d),
@@ -425,7 +448,7 @@ impl<'a> Node<'a> {
 	}
 
 	pub fn into_owned_dir(self) -> Result<OwnedDir<'a>> {
-		let resolved = self.resolve()?;
+		let resolved = self.resolve_exists()?;
 		Ok(OwnedDir { handle: OwningHandle::try_new(Box::new(resolved), |x| unsafe { (*x).as_dir().map(|x| Box::new(x)) })? })
 	}
 
@@ -472,7 +495,7 @@ pub struct Archive {
 	file: ManagedPointer<sqfs_file_t>,
 	superblock: sqfs_super_t,
 	compressor_config: sqfs_compressor_config_t,
-	mmap: (std::fs::File, MemoryMap),
+	mmap: (std::fs::File, Mmap),
 }
 
 impl Archive {
@@ -488,10 +511,12 @@ impl Archive {
 			sqfs_compressor_config_init(x, superblock.compression_id as u32, superblock.block_size as u64, SQFS_COMP_FLAG_SQFS_COMP_FLAG_UNCOMPRESS as u16)
 		}, "Couldn't read archive compressor config")?;
 		let os_file = std::fs::File::open(&path)?;
-		let map = MemoryMap::new(superblock.bytes_used as usize, &vec![MapOption::MapReadable, MapOption::MapFd(os_file.as_raw_fd())])?;
+		let map = unsafe { MmapOptions::new().map(&os_file).map_err(|e| SquashfsError::Mmap(e))? };
+		//let map = MemoryMap::new(superblock.bytes_used as usize, &vec![MapOption::MapReadable, MapOption::MapFd(os_file.as_raw_fd())])?;
 		Ok(Self { path: path.as_ref().to_path_buf(), file: file, superblock: superblock, compressor_config: compressor_config, mmap: (os_file, map) })
 	}
 
+
 	fn compressor(&self) -> Result<ManagedPointer<sqfs_compressor_t>> {
 		Ok(sfs_init_ptr(&|x| unsafe {
 			sqfs_compressor_create(&self.compressor_config, x)
@@ -506,8 +531,7 @@ impl Archive {
 	}
 
 	fn id_lookup(&self, idx: u16) -> Result<u32> {
-		// TODO Consider chaching the ID table to make lookups more efficient
-		let mut id_table = sfs_init_check_null(&|| unsafe {
+		let id_table = sfs_init_check_null(&|| unsafe {
 			sqfs_id_table_create(0)
 		}, "Couldn't create ID table", sfs_destroy)?;
 		let compressor = self.compressor()?;
@@ -517,6 +541,10 @@ impl Archive {
 		}, "Couldn't get ID from ID table")?)
 	}
 
+	pub fn path(&self) -> &Path {
+		&self.path
+	}
+
 	pub fn size(&self) -> u32 {
 		self.superblock.inode_count
 	}
@@ -550,7 +578,7 @@ impl Archive {
 		if self.superblock.flags & SQFS_SUPER_FLAGS_SQFS_FLAG_EXPORTABLE as u16 == 0 { Err(SquashfsError::Unsupported("inode indexing".to_string()))?; }
 		if id <= 0 || id > self.superblock.inode_count as u64 { Err(SquashfsError::Range(id, self.superblock.inode_count as u64))? }
 		let compressor = self.compressor()?;
-		let export_reader = self.meta_reader(&compressor, None)?; // TODO Would be nice if we could set bounds for this
+		let export_reader = self.meta_reader(&compressor, None)?; // Would be nice if we could set bounds for this
 		let (block, offset) = ((id - 1) / 1024, (id - 1) % 1024 * 8);
 		let block_start: u64 = sfs_init(&|x| unsafe {
 			let read_at = (**self.file).read_at.expect("File object does not implement read_at");
@@ -570,8 +598,7 @@ impl Archive {
 	}
 
 	fn map_range(&self, start: usize, len: usize) -> &[u8] {
-		let map = &self.mmap.1;
-		unsafe { std::slice::from_raw_parts(map.data().offset(start as isize), len) }
+		&(self.mmap.1)[start..start + len]
 	}
 }
 
diff --git a/src/write.rs b/src/write.rs
index 2eb7984..fb15ed1 100644
--- a/src/write.rs
+++ b/src/write.rs
@@ -1,27 +1,27 @@
 use std::cell::RefCell;
 use std::collections::{BTreeMap, HashMap};
-use std::ffi::{CStr, CString, OsStr, OsString};
+use std::ffi::{CString, OsString};
 use std::io::Read;
 use std::path::{Path, PathBuf};
+use std::sync::{Mutex, RwLock};
 use std::time::SystemTime;
-use bindings::*;
 use super::*;
 use super::SquashfsError;
-use thiserror::Error;
 use walkdir::{DirEntry, WalkDir};
 
-pub mod BlockFlags {
-	pub const DontCompress: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_COMPRESS;
-	pub const BlockAlign: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_ALIGN;
-	pub const DontFragment: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_FRAGMENT;
-	pub const DontDeduplicate: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_DEDUPLICATE;
-	pub const IgnoreSparse: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_IGNORE_SPARSE;
-	pub const DontHash: u32 = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_HASH;
+#[repr(u32)]
+pub enum BlockFlags {
+	DontCompress = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_COMPRESS,
+	BlockAlign = super::SQFS_BLK_FLAGS_SQFS_BLK_ALIGN,
+	DontFragment = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_FRAGMENT,
+	DontDeduplicate = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_DEDUPLICATE,
+	IgnoreSparse = super::SQFS_BLK_FLAGS_SQFS_BLK_IGNORE_SPARSE,
+	DontHash = super::SQFS_BLK_FLAGS_SQFS_BLK_DONT_HASH,
 }
 
 pub enum SourceData {
-	File(Box<dyn Read>),
-	Dir(Box<dyn Iterator<Item=(OsString, u32)>>),
+	File(Box<dyn Read + Sync + Send>),
+	Dir(Box<dyn Iterator<Item=(OsString, u32)> + Sync + Send>),
 	Symlink(OsString),
 	BlockDev(u32, u32),
 	CharDev(u32, u32),
@@ -41,18 +41,13 @@ pub struct Source {
 
 fn file_xattrs(path: &Path) -> Result<HashMap<OsString, Vec<u8>>> {
 	xattr::list(path)?.map(|attr| {
-		match xattr::get(path, attr.clone()) {
-			Err(e) => panic!(), // TODO Panics
-			Ok(None) => panic!(), //Err(anyhow!("Couldn't retrieve xattr \"{:?}\" reported to be present", attr)),
-			Ok(Some(value)) => Ok((attr, value))
-		}
+		let value = xattr::get(path, attr.clone()).map_err(|e| SquashfsError::Xattr(path.to_path_buf(), e))?
+			.expect(&format!("Could not retrieve xattr {:?} reported to be present", attr));
+		Ok((attr, value))
 	}).collect()
 }
 
 fn copy_metadata(src: &ManagedPointer<sqfs_inode_generic_t>, dst: &mut ManagedPointer<sqfs_inode_generic_t>) -> Result<()> {
-	fn nlink_ref(inode: &ManagedPointer<sqfs_inode_generic_t>) -> Option<&u32> {
-		unimplemented!();
-	}
 	let (src_base, dst_base) = unsafe { (&(***src).base, &mut (***dst).base) };
 	dst_base.mode = src_base.mode;
 	dst_base.uid_idx = src_base.uid_idx;
@@ -77,63 +72,56 @@ impl Source {
 	}
 
 	// TODO Handle hard links
-	fn to_inode(&self, link_count: u32) -> Result<ManagedPointer<sqfs_inode_generic_t>> {
-		fn create_inode(kind: SQFS_INODE_TYPE, extra: usize) -> ManagedPointer<sqfs_inode_generic_t> {
+	unsafe fn to_inode(&self, link_count: u32) -> Result<ManagedPointer<sqfs_inode_generic_t>> {
+		unsafe fn create_inode(kind: SQFS_INODE_TYPE, extra: usize) -> ManagedPointer<sqfs_inode_generic_t> {
 			use std::alloc::{alloc, Layout};
 			use std::mem::{align_of, size_of};
-			unsafe {
-				let layout = Layout::from_size_align_unchecked(size_of::<sqfs_inode_generic_t>() + extra, align_of::<sqfs_inode_generic_t>());
-				let ret = alloc(layout) as *mut sqfs_inode_generic_t;
-				(*ret).base.type_ = kind as u16;
-				ManagedPointer::new(ret, rust_dealloc)
-			}
+			let layout = Layout::from_size_align_unchecked(size_of::<sqfs_inode_generic_t>() + extra, align_of::<sqfs_inode_generic_t>());
+			let ret = alloc(layout) as *mut sqfs_inode_generic_t;
+			(*ret).base.type_ = kind as u16;
+			ManagedPointer::new(ret, rust_dealloc)
 		}
-		let ret = unsafe {
-			match &self.data {
-				SourceData::File(_) => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_FILE, 0);
-					ret
-				},
-				SourceData::Dir(_) => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_DIR, 0);
-					(**ret).data.dir.nlink = link_count;
-					ret
-				},
-				SourceData::Symlink(dest_os) => {
-					let dest = os_to_string(&dest_os)?.into_bytes();
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_SLINK, dest.len());
-					let mut data = &mut (**ret).data.slink;
-					data.nlink = link_count;
-					data.target_size = dest.len() as u32;
-					let dest_field = std::mem::transmute::<_, &mut [u8]>((**ret).extra.as_mut_slice(dest.len()));
-					dest_field.copy_from_slice(dest.as_slice());
-					ret
-				},
-				SourceData::BlockDev(maj, min) => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_BDEV, 0);
-					let mut data = &mut (**ret).data.dev;
-					data.nlink = link_count;
-					data.devno = Self::devno(*maj, *min);
-					ret
-				},
-				SourceData::CharDev(maj, min) => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_CDEV, 0);
-					let mut data = &mut (**ret).data.dev;
-					data.nlink = link_count;
-					data.devno = Self::devno(*maj, *min);
-					ret
-				},
-				SourceData::Fifo => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_FIFO, 0);
-					(**ret).data.ipc.nlink = link_count;
-					ret
-				},
-				SourceData::Socket => {
-					let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_SOCKET, 0);
-					(**ret).data.ipc.nlink = link_count;
-					ret
-				},
-			}
+		let ret = match &self.data {
+			SourceData::File(_) => create_inode(SQFS_INODE_TYPE_SQFS_INODE_FILE, 0),
+			SourceData::Dir(_) => {
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_DIR, 0);
+				(**ret).data.dir.nlink = link_count;
+				ret
+			},
+			SourceData::Symlink(dest_os) => {
+				let dest = os_to_string(&dest_os)?.into_bytes();
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_SLINK, dest.len());
+				let mut data = &mut (**ret).data.slink;
+				data.nlink = link_count;
+				data.target_size = dest.len() as u32;
+				let dest_field = std::mem::transmute::<_, &mut [u8]>((**ret).extra.as_mut_slice(dest.len()));
+				dest_field.copy_from_slice(dest.as_slice());
+				ret
+			},
+			SourceData::BlockDev(maj, min) => {
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_BDEV, 0);
+				let mut data = &mut (**ret).data.dev;
+				data.nlink = link_count;
+				data.devno = Self::devno(*maj, *min);
+				ret
+			},
+			SourceData::CharDev(maj, min) => {
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_CDEV, 0);
+				let mut data = &mut (**ret).data.dev;
+				data.nlink = link_count;
+				data.devno = Self::devno(*maj, *min);
+				ret
+			},
+			SourceData::Fifo => {
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_FIFO, 0);
+				(**ret).data.ipc.nlink = link_count;
+				ret
+			},
+			SourceData::Socket => {
+				let mut ret = create_inode(SQFS_INODE_TYPE_SQFS_INODE_SOCKET, 0);
+				(**ret).data.ipc.nlink = link_count;
+				ret
+			},
 		};
 		Ok(ret)
 	}
@@ -141,7 +129,7 @@ impl Source {
 
 struct IntermediateNode {
 	inode: Box<ManagedPointer<sqfs_inode_generic_t>>,
-	dir_children: Option<Box<dyn Iterator<Item=(OsString, u32)>>>,
+	dir_children: Option<Box<dyn Iterator<Item=(OsString, u32)> + Sync + Send>>,
 	pos: u64,
 }
 
@@ -152,25 +140,26 @@ pub struct SourceFile {
 
 pub struct Writer {
 	outfile: ManagedPointer<sqfs_file_t>,
-	compressor_config: sqfs_compressor_config_t,
+	#[allow(dead_code)] compressor_config: sqfs_compressor_config_t, // Referenced by `compressor`
 	compressor: ManagedPointer<sqfs_compressor_t>,
 	superblock: sqfs_super_t,
-	block_writer: ManagedPointer<sqfs_block_writer_t>,
-	block_processor: ManagedPointer<sqfs_block_processor_t>,
+	#[allow(dead_code)] block_writer: ManagedPointer<sqfs_block_writer_t>, // Referenced by `block_processor`
+	block_processor: Mutex<ManagedPointer<sqfs_block_processor_t>>,
 	frag_table: ManagedPointer<sqfs_frag_table_t>,
-	id_table: ManagedPointer<sqfs_id_table_t>,
-	xattr_writer: ManagedPointer<sqfs_xattr_writer_t>,
+	id_table: Mutex<ManagedPointer<sqfs_id_table_t>>,
+	xattr_writer: Mutex<ManagedPointer<sqfs_xattr_writer_t>>,
 	inode_writer: ManagedPointer<sqfs_meta_writer_t>,
 	dirent_writer: ManagedPointer<sqfs_meta_writer_t>,
 	dir_writer: ManagedPointer<sqfs_dir_writer_t>,
-	nodes: Vec<RefCell<IntermediateNode>>,
+	nodes: Mutex<Vec<RefCell<IntermediateNode>>>,
+	finished: RwLock<bool>,
 }
 
 impl Writer {
 	pub fn open<T: AsRef<Path>>(path: T) -> Result<Self> {
 		let cpath = CString::new(os_to_string(path.as_ref().as_os_str())?)?;
 		let block_size = SQFS_DEFAULT_BLOCK_SIZE as u64;
-		let num_workers = 4; // TODO Get from core count
+		let num_workers = num_cpus::get() as u32;
 		let compressor_id = SQFS_COMPRESSOR_SQFS_COMP_ZSTD;
 		let now = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH)?.as_secs() as u32;
 		let outfile = sfs_init_check_null(&|| unsafe {
@@ -191,15 +180,15 @@ impl Writer {
 		let block_writer = sfs_init_check_null(&|| unsafe {
 			sqfs_block_writer_create(*outfile, 4096, 0)
 		}, "Couldn't create block writer", sfs_destroy)?;
-		let block_processor = sfs_init_check_null(&|| unsafe {
+		let block_processor = Mutex::new(sfs_init_check_null(&|| unsafe {
 			sqfs_block_processor_create(block_size, *compressor, num_workers, 10 * num_workers as u64, *block_writer, *frag_table)
-		}, "Couldn't create block processor", sfs_destroy)?;
-		let id_table = sfs_init_check_null(&|| unsafe {
+		}, "Couldn't create block processor", sfs_destroy)?);
+		let id_table = Mutex::new(sfs_init_check_null(&|| unsafe {
 			sqfs_id_table_create(0)
-		}, "Couldn't create ID table", sfs_destroy)?;
-		let xattr_writer = sfs_init_check_null(&|| unsafe {
+		}, "Couldn't create ID table", sfs_destroy)?);
+		let xattr_writer = Mutex::new(sfs_init_check_null(&|| unsafe {
 			sqfs_xattr_writer_create(0)
-		}, "Couldn't create xattr writer", sfs_destroy)?;
+		}, "Couldn't create xattr writer", sfs_destroy)?);
 		let inode_writer = sfs_init_check_null(&|| unsafe {
 			sqfs_meta_writer_create(*outfile, *compressor, 0)
 		}, "Couldn't create inode metadata writer", sfs_destroy)?;
@@ -226,7 +215,8 @@ impl Writer {
 			inode_writer: inode_writer,
 			dirent_writer: dirent_writer,
 			dir_writer: dir_writer,
-			nodes: vec![],
+			nodes: Mutex::new(vec![]),
+			finished: RwLock::new(false),
 		})
 	}
 
@@ -257,64 +247,70 @@ impl Writer {
 		unsafe { (**self.outfile).get_size.expect("Superblock doesn't provide get_size")(*self.outfile) }
 	}
 
-	// TODO Minimize unsafe blocks
 	pub fn add(&mut self, mut source: Source) -> Result<u32> {
+		let finished = self.finished.read().expect("Poisoned lock");
+		if *finished { Err(SquashfsError::Finished)?; }
 		let flags = source.flags;
 		let nlink = 1; // TODO Handle hard links
-		let mut inode = match source.data {
-			SourceData::File(ref mut reader) => {
-				let mut ret = Box::new(ManagedPointer::null(libc_free));
-				unsafe {
-					sfs_check(sqfs_block_processor_begin_file(*self.block_processor, &mut **ret, ptr::null_mut(), flags), "Couldn't begin writing file")?;
+		let mut inode = unsafe {
+			match source.data {
+				SourceData::File(ref mut reader) => {
+					let mut ret = Box::new(ManagedPointer::null(libc_free));
+					let block_processor = self.block_processor.lock().expect("Poisoned lock");
+					sfs_check(sqfs_block_processor_begin_file(**block_processor, &mut **ret, ptr::null_mut(), flags), "Couldn't begin writing file")?;
 					let mut buf = vec![0; BLOCK_BUF_SIZE];
 					loop {
 						let rdsize = reader.read(&mut buf)? as u64;
 						if rdsize == 0 { break; }
-						sfs_check(sqfs_block_processor_append(*self.block_processor, &buf as &[u8] as *const [u8] as *const libc::c_void, rdsize), "Couldn't write file data block")?;
+						sfs_check(sqfs_block_processor_append(**block_processor, &buf as &[u8] as *const [u8] as *const libc::c_void, rdsize), "Couldn't write file data block")?;
 					}
-					sfs_check(sqfs_block_processor_end_file(*self.block_processor), "Couldn't finish writing file")?;
-				}
-				ret
-			},
-			_ => Box::new(source.to_inode(nlink)?),
+					sfs_check(sqfs_block_processor_end_file(**block_processor), "Couldn't finish writing file")?;
+					ret
+				},
+				_ => Box::new(source.to_inode(nlink)?),
+			}
 		};
 		unsafe {
-			sfs_check(sqfs_xattr_writer_begin(*self.xattr_writer, 0), "Couldn't start writing xattrs")?;
+			let xattr_writer = self.xattr_writer.lock().expect("Poisoned lock");
+			sfs_check(sqfs_xattr_writer_begin(**xattr_writer, 0), "Couldn't start writing xattrs")?;
 			for (key, value) in &source.xattrs {
 				let ckey = CString::new(os_to_string(key)?)?;
-				sfs_check(sqfs_xattr_writer_add(*self.xattr_writer, ckey.as_ptr() as *const i8, value.as_ptr() as *const libc::c_void, value.len() as u64), "Couldn't add xattr")?;
+				sfs_check(sqfs_xattr_writer_add(**xattr_writer, ckey.as_ptr() as *const i8, value.as_ptr() as *const libc::c_void, value.len() as u64), "Couldn't add xattr")?;
 			}
-			let xattr_idx = unsafe { sfs_init(&|x| sqfs_xattr_writer_end(*self.xattr_writer, x), "Couldn't finish writing xattrs")? };
+			let xattr_idx = sfs_init(&|x| sqfs_xattr_writer_end(**xattr_writer, x), "Couldn't finish writing xattrs")?;
 			let mut base = &mut (***inode).base;
 			base.mode = source.mode;
 			sqfs_inode_set_xattr_index(**inode, xattr_idx);
-			sfs_check(sqfs_id_table_id_to_index(*self.id_table, source.uid, &mut base.uid_idx), "Couldn't set inode UID")?;
-			sfs_check(sqfs_id_table_id_to_index(*self.id_table, source.gid, &mut base.gid_idx), "Couldn't set inode GID")?;
+			let id_table = self.id_table.lock().expect("Poisoned lock");
+			sfs_check(sqfs_id_table_id_to_index(**id_table, source.uid, &mut base.uid_idx), "Couldn't set inode UID")?;
+			sfs_check(sqfs_id_table_id_to_index(**id_table, source.gid, &mut base.gid_idx), "Couldn't set inode GID")?;
 			base.mod_time = source.modified;
-			base.inode_number = self.nodes.len() as u32 + 1;
 		}
 		let dir_children = match source.data {
 			SourceData::Dir(children) => Some(children),
 			_ => None,
 		};
-		self.nodes.push(RefCell::new(IntermediateNode { inode: inode, dir_children: dir_children, pos: 0 }));
-		Ok(self.nodes.len() as u32)
+		let mut nodes = self.nodes.lock().expect("Poisoned lock");
+		unsafe { (***inode).base.inode_number = nodes.len() as u32 + 1; }
+		nodes.push(RefCell::new(IntermediateNode { inode: inode, dir_children: dir_children, pos: 0 }));
+		Ok(nodes.len() as u32)
 	}
 
 	pub fn finish(&mut self) -> Result<()> {
+		*self.finished.write().expect("Poisoned lock") = true;
+		let nodes = self.nodes.lock().expect("Poisoned lock");
 		unsafe {
-			sfs_check(sqfs_block_processor_finish(*self.block_processor), "Failed finishing block processing")?;
+			sfs_check(sqfs_block_processor_finish(**self.block_processor.lock().expect("Poisoned lock")), "Failed finishing block processing")?;
 			self.superblock.inode_table_start = self.outfile_size();
-			for raw_node in &self.nodes {
+			for raw_node in &*nodes {
 				let mut node = raw_node.borrow_mut();
-				// TODO Handle extended inodes properly
 				let id = (***node.inode).base.inode_number;
 				if let Some(children) = node.dir_children.take() {
 					sfs_check(sqfs_dir_writer_begin(*self.dir_writer, 0), "Couldn't start writing directory")?;
 					// For each child, need: name, ID, reference, mode
-					for (name, child_id) in children { // TODO Check that children are sorted
+					for (name, child_id) in children { // On disk children need to be sorted -- I think the library takes care of this
 						if child_id >= id { Err(SquashfsError::WriteOrder(id, child_id))?; }
-						let child_node = &self.nodes[child_id as usize - 1].borrow();
+						let child_node = &nodes[child_id as usize - 1].borrow();
 						let child = child_node.inode.as_ref();
 						let child_ref = child_node.pos;
 						sfs_check(sqfs_dir_writer_add_entry(*self.dir_writer, CString::new(os_to_string(&name)?)?.as_ptr(), child_id, child_ref, Self::mode_from_inode(&child)), "Couldn't add directory entry")?;
@@ -323,7 +319,7 @@ impl Writer {
 					let mut ret = Box::new(sfs_init_check_null(&|| {
 						sqfs_dir_writer_create_inode(*self.dir_writer, 0, 0, 0) // TODO Populate the parent inode number (how?)
 					}, "Couldn't get inode for directory", libc_free)?);
-					copy_metadata(&*node.inode, &mut ret);
+					copy_metadata(&*node.inode, &mut ret)?;
 					node.inode = ret;
 				}
 				let (mut block, mut offset) = (0, 0);
@@ -332,46 +328,49 @@ impl Writer {
 				sfs_check(sqfs_meta_writer_write_inode(*self.inode_writer, **node.inode), "Couldn't write inode")?;
 			}
 
-			let root_ref = self.nodes.last().ok_or(SquashfsError::Empty)?.borrow().pos;
+			let root_ref = nodes.last().ok_or(SquashfsError::Empty)?.borrow().pos;
 			self.superblock.root_inode_ref = root_ref;
 			sfs_check(sqfs_meta_writer_flush(*self.inode_writer), "Couldn't flush inodes")?;
 			sfs_check(sqfs_meta_writer_flush(*self.dirent_writer), "Couldn't flush directory entries")?;
 			self.superblock.directory_table_start = self.outfile_size();
 			sfs_check(sqfs_meta_write_write_to_file(*self.dirent_writer), "Couldn't write directory entries")?;
-			self.superblock.inode_count = self.nodes.len() as u32;
+			self.superblock.inode_count = nodes.len() as u32;
 			sfs_check(sqfs_frag_table_write(*self.frag_table, *self.outfile, &mut self.superblock, *self.compressor), "Couldn't write fragment table")?;
-			sfs_check(sqfs_dir_writer_write_export_table(*self.dir_writer, *self.outfile, *self.compressor, self.nodes.len() as u32, root_ref, &mut self.superblock), "Couldn't write export table")?;
-			sfs_check(sqfs_id_table_write(*self.id_table, *self.outfile, &mut self.superblock, *self.compressor), "Couldn't write ID table")?;
-			sfs_check(sqfs_xattr_writer_flush(*self.xattr_writer, *self.outfile, &mut self.superblock, *self.compressor), "Couldn't write xattr table")?;
+			sfs_check(sqfs_dir_writer_write_export_table(*self.dir_writer, *self.outfile, *self.compressor, nodes.len() as u32, root_ref, &mut self.superblock), "Couldn't write export table")?;
+			sfs_check(sqfs_id_table_write(**self.id_table.lock().expect("Poisoned lock"), *self.outfile, &mut self.superblock, *self.compressor), "Couldn't write ID table")?;
+			sfs_check(sqfs_xattr_writer_flush(**self.xattr_writer.lock().expect("Poisoned lock"), *self.outfile, &mut self.superblock, *self.compressor), "Couldn't write xattr table")?;
 			self.superblock.bytes_used = self.outfile_size();
 			sfs_check(sqfs_super_write(&self.superblock, *self.outfile), "Couldn't rewrite archive superblock")?;
 			let padding: Vec<u8> = vec![0; PAD_TO - self.outfile_size() as usize % PAD_TO];
-			sfs_check((**self.outfile).write_at.expect("File does not provide write_at")(*self.outfile, self.outfile_size(), &padding as &[u8] as *const [u8] as *const libc::c_void, padding.len() as u64), "Couldn't pad file");
+			sfs_check((**self.outfile).write_at.expect("File does not provide write_at")(*self.outfile, self.outfile_size(), &padding as &[u8] as *const [u8] as *const libc::c_void, padding.len() as u64), "Couldn't pad file")?;
 		}
 		Ok(())
 	}
 }
 
+unsafe impl Sync for Writer { }
+unsafe impl Send for Writer { }
+
 pub struct TreeProcessor {
 	root: PathBuf,
-	writer: RefCell<Writer>,
-	childmap: RefCell<HashMap<PathBuf, BTreeMap<OsString, u32>>>,
+	writer: Mutex<Writer>,
+	childmap: Mutex<HashMap<PathBuf, BTreeMap<OsString, u32>>>,
 }
 
 impl TreeProcessor {
 	pub fn new<P: AsRef<Path>>(writer: Writer, root: P) -> Result<Self> {
-		Ok(Self { root: root.as_ref().to_path_buf(), writer: RefCell::new(writer), childmap: RefCell::new(HashMap::new()) })
+		Ok(Self { root: root.as_ref().to_path_buf(), writer: Mutex::new(writer), childmap: Mutex::new(HashMap::new()) })
 	}
 
 	pub fn add(&self, mut source: SourceFile) -> Result<u32> {
-		let mut childmap = self.childmap.borrow_mut();
+		let mut childmap = self.childmap.lock().expect("Poisoned lock");
 		if let SourceData::Dir(children) = &mut source.content.data {
 			// Pull in any last-minute additions made by the user
 			let mut new_children = childmap.remove(&source.path).unwrap_or(BTreeMap::new());
 			new_children.extend(children);
 			source.content.data = SourceData::Dir(Box::new(new_children.into_iter()));
 		}
-		let id = self.writer.borrow_mut().add(source.content)?;
+		let id = self.writer.lock().expect("Poisoned lock").add(source.content)?;
 		if let Some(parent) = source.path.parent() {
 			childmap.entry(parent.to_path_buf()).or_insert(BTreeMap::new()).insert(source.path.file_name().expect("Path from walkdir has no file name").to_os_string(), id);
 		}
@@ -379,16 +378,14 @@ impl TreeProcessor {
 	}
 
 	pub fn finish(&self) -> Result<()> {
-		self.writer.borrow_mut().finish()
+		self.writer.lock().expect("Poisoned lock").finish()
 	}
 
 	fn make_source(&self, entry: DirEntry) -> Result<Source> {
-		// TODO Consider adding Unix-specific functionality with graceful degradation
-		// TODO Catch all errors except add() and continue
 		let metadata = entry.metadata().unwrap();
 		let mtime = metadata.modified()?.duration_since(SystemTime::UNIX_EPOCH)?.as_secs() as u32;
 		let data = if metadata.file_type().is_dir() {
-			SourceData::Dir(Box::new(self.childmap.borrow_mut().remove(&entry.path().to_path_buf()).unwrap_or(BTreeMap::new()).into_iter()))
+			SourceData::Dir(Box::new(self.childmap.lock().expect("Poisoned lock").remove(&entry.path().to_path_buf()).unwrap_or(BTreeMap::new()).into_iter()))
 		}
 		else if metadata.file_type().is_file() {
 			SourceData::File(Box::new(std::fs::File::open(entry.path())?))
@@ -404,6 +401,10 @@ impl TreeProcessor {
 			use std::os::linux::fs::MetadataExt;
 			Source { data: data, xattrs: file_xattrs(entry.path())?, uid: metadata.st_uid(), gid: metadata.st_gid(), mode: (metadata.st_mode() & !S_IFMT) as u16, modified: mtime, flags: 0 }
 		}
+		else if cfg!(unix) {
+			use std::os::unix::fs::MetadataExt;
+			Source { data: data, xattrs: HashMap::new(), uid: metadata.uid(), gid: metadata.gid(), mode: (metadata.mode() & 0x1ff) as u16, modified: mtime, flags: 0 }
+		}
 		else {
 			Source { data: data, xattrs: HashMap::new(), uid: 0, gid: 0, mode: 0x1ff, modified: mtime, flags: 0 }
 		};