diff --git a/crates/librqbit/src/storage/filesystem/fs.rs b/crates/librqbit/src/storage/filesystem/fs.rs index d4051a753..0a6da217f 100644 --- a/crates/librqbit/src/storage/filesystem/fs.rs +++ b/crates/librqbit/src/storage/filesystem/fs.rs @@ -62,10 +62,11 @@ impl TorrentStorage for FilesystemStorage { { use std::os::unix::fs::FileExt; Ok(of - .file + .file_handle .read() .as_ref() .context("file is None")? + .file .read_exact_at(buf, offset)?) } #[cfg(target_family = "windows")] @@ -88,22 +89,24 @@ impl TorrentStorage for FilesystemStorage { fn pwrite_all(&self, file_id: usize, offset: u64, buf: &[u8]) -> anyhow::Result<()> { let of = self.opened_files.get(file_id).context("no such file")?; + of.ensure_writeable()?; #[cfg(target_family = "unix")] { use std::os::unix::fs::FileExt; Ok(of - .file + .file_handle .read() .as_ref() .context("file is None")? + .file .write_all_at(buf, offset)?) } #[cfg(target_family = "windows")] { use std::os::windows::fs::FileExt; let mut remaining = buf.len(); - let g = of.file.read(); - let f = g.as_ref().context("file is None")?; + let g = of.file_handle.read(); + let f = g.as_ref().context("file is None")?.file; while remaining > 0 { remaining -= f.seek_write(buf, offset)?; } @@ -112,8 +115,8 @@ impl TorrentStorage for FilesystemStorage { #[cfg(not(any(target_family = "unix", target_family = "windows")))] { use std::io::{Read, Seek, SeekFrom, Write}; - let mut g = of.file.write(); - let mut f = g.as_ref().context("file is None")?; + let mut g = of.file_handle.write(); + let mut f = g.as_ref().context("file is None")?.file; f.seek(SeekFrom::Start(offset))?; Ok(f.write_all(buf)?) } @@ -125,10 +128,11 @@ impl TorrentStorage for FilesystemStorage { fn ensure_file_length(&self, file_id: usize, len: u64) -> anyhow::Result<()> { Ok(self.opened_files[file_id] - .file + .file_handle .write() .as_ref() .context("file is None")? + .file .set_len(len)?) } @@ -148,7 +152,7 @@ impl TorrentStorage for FilesystemStorage { if !path.is_dir() { anyhow::bail!("cannot remove dir: {path:?} is not a directory") } - if std::fs::read_dir(&path)?.count() == 0 { + if std::fs::read_dir(&path)?.next().is_none() { std::fs::remove_dir(&path).with_context(|| format!("error removing {path:?}")) } else { warn!("did not remove {path:?} as it was not empty"); @@ -172,17 +176,34 @@ impl TorrentStorage for FilesystemStorage { continue; }; std::fs::create_dir_all(full_path.parent().context("bug: no parent")?)?; - let f = if shared.options.allow_overwrite { - OpenOptions::new() + if shared.options.allow_overwrite { + // ensure file exists + let (file, writeable) = match OpenOptions::new() .create(true) - .truncate(false) .read(true) .write(true) + .append(false) + .truncate(false) .open(&full_path) - .with_context(|| format!("error opening {full_path:?} in read/write mode"))? + { + Ok(file) => (file, true), + Err(e) => { + warn!(?full_path, "error opening file in create+write mode: {e:?}"); + // open the file in read-only mode, will reopen in write mode later. + ( + OpenOptions::new() + .create(false) + .read(true) + .open(&full_path) + .with_context(|| format!("error opening {full_path:?}"))?, + false, + ) + } + }; + files.push(OpenedFile::new(full_path.clone(), file, writeable)); } else { // create_new does not seem to work with read(true), so calling this twice. - OpenOptions::new() + let file = OpenOptions::new() .create_new(true) .write(true) .open(&full_path) @@ -192,9 +213,9 @@ impl TorrentStorage for FilesystemStorage { &full_path ) })?; - OpenOptions::new().read(true).write(true).open(&full_path)? + OpenOptions::new().read(true).write(true).open(&full_path)?; + files.push(OpenedFile::new(full_path.clone(), file, true)); }; - files.push(OpenedFile::new(f)); } self.opened_files = files; diff --git a/crates/librqbit/src/storage/filesystem/mmap.rs b/crates/librqbit/src/storage/filesystem/mmap.rs index a1824c677..b3945e70c 100644 --- a/crates/librqbit/src/storage/filesystem/mmap.rs +++ b/crates/librqbit/src/storage/filesystem/mmap.rs @@ -109,11 +109,11 @@ impl TorrentStorage for MmapFilesystemStorage { self.fs.init(shared, metadata)?; let mut mmaps = Vec::new(); for (idx, file) in self.fs.opened_files.iter().enumerate() { - let fg = file.file.write(); - let fg = fg.as_ref().context("file is None")?; - fg.set_len(metadata.file_infos[idx].len) + let mut fh = file.file_handle.write(); + let file = &fh.as_mut().context("file is None")?.file; + file.set_len(metadata.file_infos[idx].len) .context("mmap storage: error setting length")?; - let mmap = unsafe { MmapOptions::new().map_mut(fg) }.context("error mapping file")?; + let mmap = unsafe { MmapOptions::new().map_mut(file) }.context("error mapping file")?; mmaps.push(RwLock::new(mmap)); } diff --git a/crates/librqbit/src/storage/filesystem/opened_file.rs b/crates/librqbit/src/storage/filesystem/opened_file.rs index 051c652e1..35be70387 100644 --- a/crates/librqbit/src/storage/filesystem/opened_file.rs +++ b/crates/librqbit/src/storage/filesystem/opened_file.rs @@ -1,34 +1,82 @@ -use std::fs::File; +use std::{ + fs::File, + fs::OpenOptions, + path::PathBuf, +}; + +use anyhow::Context; use parking_lot::RwLock; +#[derive(Debug)] +pub(crate) struct FileHandle { + pub file: File, + pub is_writeable: bool, +} + #[derive(Debug)] pub(crate) struct OpenedFile { - pub file: RwLock>, + pub filename: PathBuf, + pub file_handle: RwLock>, } impl OpenedFile { - pub fn new(f: File) -> Self { - Self { - file: RwLock::new(Some(f)), - } + pub fn new(filename: PathBuf, file: File, is_writeable: bool) -> Self { + let file_handle = RwLock::new(Some(FileHandle { + file, + is_writeable, + })); + Self { filename, file_handle } } pub fn new_dummy() -> Self { Self { - file: RwLock::new(None), + filename: PathBuf::new(), + file_handle: None.into(), } } pub fn take(&self) -> anyhow::Result> { - let mut f = self.file.write(); - Ok(f.take()) + let mut fh = self.file_handle.write(); + if let Some(file_handle) = fh.take() { + Ok(Some(file_handle.file)) + } else { + Ok(None) + } + } + + pub fn is_writeable(&self) -> bool { + self.file_handle.read().as_ref().and_then(|fh| Some(fh.is_writeable)).unwrap_or(false) } pub fn take_clone(&self) -> anyhow::Result { - let f = self.take()?; + let file = self.take().unwrap().with_context(|| format!("error taking file for {:?}", self.filename))?; Ok(Self { - file: RwLock::new(f), + filename: self.filename.clone(), + file_handle: RwLock::new( + Some(FileHandle { + file, + is_writeable: self.is_writeable(), + }) + ), }) } + + pub fn ensure_writeable(&self) -> anyhow::Result<()> { + let mut fh = self.file_handle.write(); + if let Some(file_handle) = fh.as_mut() { + if !file_handle.is_writeable { + let new_file = OpenOptions::new() + .write(true) + .create(false) + .open(&self.filename) + .with_context(|| format!("error opening {:?} in write mode", self.filename))?; + *file_handle = FileHandle { + file: new_file, + is_writeable: true, + }; + } + } + Ok(()) + } }