diff --git a/Cargo.lock b/Cargo.lock index 1bc772bd5c1..6c25477d169 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5068,6 +5068,7 @@ dependencies = [ "derivative", "filetime", "fs_extra", + "futures", "getrandom", "indexmap", "lazy_static", @@ -6256,6 +6257,7 @@ name = "wasmer-wast" version = "4.0.0-beta.2" dependencies = [ "anyhow", + "futures", "serde", "tempfile", "thiserror", diff --git a/lib/virtual-fs/Cargo.toml b/lib/virtual-fs/Cargo.toml index c958428cce1..d9c1f6ef701 100644 --- a/lib/virtual-fs/Cargo.toml +++ b/lib/virtual-fs/Cargo.toml @@ -12,6 +12,7 @@ rust-version.workspace = true [dependencies] libc = { version = "^0.2", default-features = false, optional = true } thiserror = "1" +futures = { version = "0.3" } tracing = { version = "0.1" } typetag = { version = "0.1", optional = true } webc = { version = "5.0", optional = true } diff --git a/lib/virtual-fs/src/arc_box_file.rs b/lib/virtual-fs/src/arc_box_file.rs index a4563ba87f8..48a8d405fc8 100644 --- a/lib/virtual-fs/src/arc_box_file.rs +++ b/lib/virtual-fs/src/arc_box_file.rs @@ -1,16 +1,19 @@ //! Used for sharing references to the same file across multiple file systems, //! effectively this is a symbolic link without all the complex path redirection -use crate::VirtualFile; -use derivative::Derivative; -use std::pin::Pin; -use std::task::{Context, Poll}; use std::{ io::{self, *}, + pin::Pin, sync::{Arc, Mutex}, + task::{Context, Poll}, }; + +use derivative::Derivative; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; +use crate::VirtualFile; + #[derive(Derivative, Clone)] #[derivative(Debug)] pub struct ArcBoxFile { @@ -108,9 +111,11 @@ impl VirtualFile for ArcBoxFile { let mut inner = self.inner.lock().unwrap(); inner.set_len(new_size) } - fn unlink(&mut self) -> crate::Result<()> { + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { let mut inner = self.inner.lock().unwrap(); - inner.unlink() + let fut = inner.unlink(); + drop(inner); + Box::pin(async { fut.await }) } fn is_open(&self) -> bool { let inner = self.inner.lock().unwrap(); diff --git a/lib/virtual-fs/src/arc_file.rs b/lib/virtual-fs/src/arc_file.rs index 8ba9ee73a64..e3961d0b7d6 100644 --- a/lib/virtual-fs/src/arc_file.rs +++ b/lib/virtual-fs/src/arc_file.rs @@ -3,6 +3,7 @@ use crate::{ClonableVirtualFile, VirtualFile}; use derivative::Derivative; +use futures::future::BoxFuture; use std::pin::Pin; use std::task::{Context, Poll}; use std::{ @@ -126,9 +127,11 @@ where let mut inner = self.inner.lock().unwrap(); inner.set_len(new_size) } - fn unlink(&mut self) -> crate::Result<()> { + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { let mut inner = self.inner.lock().unwrap(); - inner.unlink() + let fut = inner.unlink(); + drop(inner); + Box::pin(async move { fut.await }) } fn is_open(&self) -> bool { let inner = self.inner.lock().unwrap(); diff --git a/lib/virtual-fs/src/arc_fs.rs b/lib/virtual-fs/src/arc_fs.rs index 3a9962049c4..734774fc16c 100644 --- a/lib/virtual-fs/src/arc_fs.rs +++ b/lib/virtual-fs/src/arc_fs.rs @@ -2,10 +2,7 @@ //! can pass clonable file systems with a `Box` to other //! interfaces -use std::path::Path; -use std::sync::Arc; -#[allow(unused_imports, dead_code)] -use tracing::{debug, error, info, trace, warn}; +use std::{path::Path, sync::Arc}; use crate::*; @@ -33,8 +30,8 @@ impl FileSystem for ArcFileSystem { self.fs.remove_dir(path) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - self.fs.rename(from, to) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { self.fs.rename(from, to).await }) } fn metadata(&self, path: &Path) -> Result { diff --git a/lib/virtual-fs/src/buffer_file.rs b/lib/virtual-fs/src/buffer_file.rs index 9af66811cbf..ba4881a72f5 100644 --- a/lib/virtual-fs/src/buffer_file.rs +++ b/lib/virtual-fs/src/buffer_file.rs @@ -5,6 +5,7 @@ use std::io::{self, *}; use std::pin::Pin; use std::task::{Context, Poll}; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use crate::VirtualFile; @@ -84,8 +85,8 @@ impl VirtualFile for BufferFile { self.data.get_mut().resize(new_size as usize, 0); Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { let cur = self.data.stream_position().unwrap_or_default(); diff --git a/lib/virtual-fs/src/combine_file.rs b/lib/virtual-fs/src/combine_file.rs index f3db42ca58c..2339d392b2c 100644 --- a/lib/virtual-fs/src/combine_file.rs +++ b/lib/virtual-fs/src/combine_file.rs @@ -41,8 +41,9 @@ impl VirtualFile for CombineFile { self.tx.set_len(new_size) } - fn unlink(&mut self) -> Result<()> { - self.tx.unlink() + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + let fut = self.tx.unlink(); + Box::pin(async { fut.await }) } fn poll_read_ready(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { diff --git a/lib/virtual-fs/src/cow_file.rs b/lib/virtual-fs/src/cow_file.rs index fd213936408..1d7e452d7ac 100644 --- a/lib/virtual-fs/src/cow_file.rs +++ b/lib/virtual-fs/src/cow_file.rs @@ -1,6 +1,7 @@ //! Used for /dev/zero - infinitely returns zero //! which is useful for commands like `dd if=/dev/zero of=bigfile.img size=1G` +use futures::future::BoxFuture; use replace_with::replace_with_or_abort; use std::io::{self, *}; use std::pin::Pin; @@ -204,9 +205,9 @@ impl VirtualFile for CopyOnWriteFile { fn set_len(&mut self, new_size: u64) -> crate::Result<()> { self.buf.set_len(new_size) } - fn unlink(&mut self) -> crate::Result<()> { - self.buf.set_len(0)?; - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + let ret = self.buf.set_len(0); + Box::pin(async move { ret }) } fn poll_read_ready(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { match self.poll_copy_progress(cx) { diff --git a/lib/virtual-fs/src/dual_write_file.rs b/lib/virtual-fs/src/dual_write_file.rs index 363e0446590..0f290294e63 100644 --- a/lib/virtual-fs/src/dual_write_file.rs +++ b/lib/virtual-fs/src/dual_write_file.rs @@ -45,12 +45,13 @@ impl VirtualFile for DualWriteFile { self.inner.size() } - fn set_len(&mut self, new_size: u64) -> Result<()> { + fn set_len(&mut self, new_size: u64) -> crate::Result<()> { self.inner.set_len(new_size) } - fn unlink(&mut self) -> Result<()> { - self.inner.unlink() + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + let fut = self.inner.unlink(); + Box::pin(async { fut.await }) } fn poll_read_ready(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { diff --git a/lib/virtual-fs/src/empty_fs.rs b/lib/virtual-fs/src/empty_fs.rs index e046f6307e6..04807ef4bad 100644 --- a/lib/virtual-fs/src/empty_fs.rs +++ b/lib/virtual-fs/src/empty_fs.rs @@ -24,8 +24,8 @@ impl FileSystem for EmptyFileSystem { Err(FsError::EntryNotFound) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - Err(FsError::EntryNotFound) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { Err(FsError::EntryNotFound) }) } fn metadata(&self, path: &Path) -> Result { diff --git a/lib/virtual-fs/src/filesystems.rs b/lib/virtual-fs/src/filesystems.rs index bf47eb2df8e..46682a49a16 100644 --- a/lib/virtual-fs/src/filesystems.rs +++ b/lib/virtual-fs/src/filesystems.rs @@ -4,7 +4,7 @@ use crate::FileSystem; pub trait FileSystems<'a>: 'a { // FIXME(Michael-F-Bryan): Rewrite this to use GATs when we bump the MSRV to // 1.65 or higher. That'll get rid of all the lifetimes and HRTBs. - type Iter: IntoIterator + 'a; + type Iter: IntoIterator + Send + 'a; /// Get something that can be used to iterate over the underlying /// filesystems. @@ -25,7 +25,7 @@ where impl<'a, T> FileSystems<'a> for Vec where - T: FileSystem, + T: FileSystem + Send, { type Iter = <[T] as FileSystems<'a>>::Iter; @@ -36,15 +36,15 @@ where impl<'a, T, const N: usize> FileSystems<'a> for [T; N] where - T: FileSystem, + T: FileSystem + Send, { - type Iter = [&'a dyn FileSystem; N]; + type Iter = [&'a (dyn FileSystem + Send); N]; fn filesystems(&'a self) -> Self::Iter { // TODO: rewrite this when array::each_ref() is stable let mut i = 0; [(); N].map(|_| { - let f = &self[i] as &dyn FileSystem; + let f = &self[i] as &(dyn FileSystem + Send); i += 1; f }) @@ -53,17 +53,17 @@ where impl<'a, T> FileSystems<'a> for [T] where - T: FileSystem, + T: FileSystem + Send, { - type Iter = std::iter::Map, fn(&T) -> &dyn FileSystem>; + type Iter = std::iter::Map, fn(&T) -> &(dyn FileSystem + Send)>; fn filesystems(&'a self) -> Self::Iter { - self.iter().map(|fs| fs as &dyn FileSystem) + self.iter().map(|fs| fs as &(dyn FileSystem + Send)) } } impl<'a> FileSystems<'a> for () { - type Iter = std::iter::Empty<&'a dyn FileSystem>; + type Iter = std::iter::Empty<&'a (dyn FileSystem + Send)>; fn filesystems(&'a self) -> Self::Iter { std::iter::empty() @@ -84,14 +84,14 @@ macro_rules! tuple_filesystems { $first: FileSystem, $($rest: FileSystem),* { - type Iter = [&'a dyn FileSystem; count!($first $($rest)*)]; + type Iter = [&'a (dyn FileSystem + Send); count!($first $($rest)*)]; fn filesystems(&'a self) -> Self::Iter { #[allow(non_snake_case)] let ($first, $($rest),*) = self; [ - $first as &dyn FileSystem, + $first as &(dyn FileSystem + Send), $($rest),* ] } diff --git a/lib/virtual-fs/src/host_fs.rs b/lib/virtual-fs/src/host_fs.rs index 03264114cda..2c42327e4fe 100644 --- a/lib/virtual-fs/src/host_fs.rs +++ b/lib/virtual-fs/src/host_fs.rs @@ -3,6 +3,7 @@ use crate::{ VirtualFile, }; use bytes::{Buf, Bytes}; +use futures::future::BoxFuture; #[cfg(feature = "enable-serde")] use serde::{de, Deserialize, Serialize}; use std::convert::TryInto; @@ -15,7 +16,6 @@ use std::task::{Context, Poll}; use std::time::{SystemTime, UNIX_EPOCH}; use tokio::fs as tfs; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite, ReadBuf}; -use tracing::debug; #[derive(Debug, Default, Clone)] #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))] @@ -67,49 +67,53 @@ impl crate::FileSystem for FileSystem { fs::remove_dir(path).map_err(Into::into) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - use filetime::{set_file_mtime, FileTime}; - if from.parent().is_none() { - return Err(FsError::BaseNotDirectory); - } - if to.parent().is_none() { - return Err(FsError::BaseNotDirectory); - } - if !from.exists() { - return Err(FsError::EntryNotFound); - } - let from_parent = from.parent().unwrap(); - let to_parent = to.parent().unwrap(); - if !from_parent.exists() { - return Err(FsError::EntryNotFound); - } - if !to_parent.exists() { - return Err(FsError::EntryNotFound); - } - let result = if from_parent != to_parent { - let _ = std::fs::create_dir_all(to_parent); - if from.is_dir() { - fs_extra::move_items( - &[from], - to, - &fs_extra::dir::CopyOptions { - copy_inside: true, - ..Default::default() - }, - ) - .map(|_| ()) - .map_err(|_| FsError::UnknownError)?; - let _ = fs_extra::remove_items(&[from]); - Ok(()) - } else { - fs::copy(from, to).map(|_| ()).map_err(FsError::from)?; - fs::remove_file(from).map(|_| ()).map_err(Into::into) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + let from = from.to_owned(); + let to = to.to_owned(); + Box::pin(async move { + use filetime::{set_file_mtime, FileTime}; + if from.parent().is_none() { + return Err(FsError::BaseNotDirectory); } - } else { - fs::rename(from, to).map_err(Into::into) - }; - let _ = set_file_mtime(to, FileTime::now()).map(|_| ()); - result + if to.parent().is_none() { + return Err(FsError::BaseNotDirectory); + } + if !from.exists() { + return Err(FsError::EntryNotFound); + } + let from_parent = from.parent().unwrap(); + let to_parent = to.parent().unwrap(); + if !from_parent.exists() { + return Err(FsError::EntryNotFound); + } + if !to_parent.exists() { + return Err(FsError::EntryNotFound); + } + let result = if from_parent != to_parent { + let _ = std::fs::create_dir_all(to_parent); + if from.is_dir() { + fs_extra::move_items( + &[&from], + &to, + &fs_extra::dir::CopyOptions { + copy_inside: true, + ..Default::default() + }, + ) + .map(|_| ()) + .map_err(|_| FsError::UnknownError)?; + let _ = fs_extra::remove_items(&[&from]); + Ok(()) + } else { + fs::copy(&from, &to).map(|_| ()).map_err(FsError::from)?; + fs::remove_file(&from).map(|_| ()).map_err(Into::into) + } + } else { + fs::rename(&from, &to).map_err(Into::into) + }; + let _ = set_file_mtime(&to, FileTime::now()).map(|_| ()); + result + }) } fn remove_file(&self, path: &Path) -> Result<()> { @@ -385,12 +389,13 @@ impl VirtualFile for File { self.metadata().len() } - fn set_len(&mut self, new_size: u64) -> Result<()> { + fn set_len(&mut self, new_size: u64) -> crate::Result<()> { fs::File::set_len(&self.inner_std, new_size).map_err(Into::into) } - fn unlink(&mut self) -> Result<()> { - fs::remove_file(&self.host_path).map_err(Into::into) + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + let path = self.host_path.clone(); + Box::pin(async move { fs::remove_file(&path).map_err(Into::into) }) } fn get_special_fd(&self) -> Option { @@ -517,13 +522,12 @@ impl VirtualFile for Stdout { 0 } - fn set_len(&mut self, _new_size: u64) -> Result<()> { - debug!("Calling VirtualFile::set_len on stdout; this is probably a bug"); - Err(FsError::PermissionDenied) + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { + Ok(()) } - fn unlink(&mut self) -> Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + Box::pin(async { Ok(()) }) } fn get_special_fd(&self) -> Option { @@ -692,13 +696,12 @@ impl VirtualFile for Stderr { 0 } - fn set_len(&mut self, _new_size: u64) -> Result<()> { - debug!("Calling VirtualFile::set_len on stderr; this is probably a bug"); - Err(FsError::PermissionDenied) + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { + Ok(()) } - fn unlink(&mut self) -> Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + Box::pin(async { Ok(()) }) } fn get_special_fd(&self) -> Option { @@ -821,13 +824,12 @@ impl VirtualFile for Stdin { fn size(&self) -> u64 { 0 } - fn set_len(&mut self, _new_size: u64) -> Result<()> { - debug!("Calling VirtualFile::set_len on stdin; this is probably a bug"); - Err(FsError::PermissionDenied) - } - fn unlink(&mut self) -> Result<()> { + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + Box::pin(async { Ok(()) }) + } fn get_special_fd(&self) -> Option { Some(0) } @@ -873,8 +875,8 @@ mod tests { use crate::FsError; use std::path::Path; - #[test] - fn test_new_filesystem() { + #[tokio::test] + async fn test_new_filesystem() { let temp = TempDir::new().unwrap(); std::fs::write(temp.path().join("foo2.txt"), b"").unwrap(); @@ -1006,8 +1008,8 @@ mod tests { .collect::>() } - #[test] - fn test_rename() { + #[tokio::test] + async fn test_rename() { let temp = TempDir::new().unwrap(); let fs = FileSystem::default(); std::fs::create_dir_all(temp.path().join("foo").join("qux")).unwrap(); @@ -1015,18 +1017,18 @@ mod tests { let bar = temp.path().join("bar"); assert_eq!( - fs.rename(Path::new("/"), Path::new("/bar")), + fs.rename(Path::new("/"), Path::new("/bar")).await, Err(FsError::BaseNotDirectory), "renaming a directory that has no parent", ); assert_eq!( - fs.rename(Path::new("/foo"), Path::new("/")), + fs.rename(Path::new("/foo"), Path::new("/")).await, Err(FsError::BaseNotDirectory), "renaming to a directory that has no parent", ); assert_eq!( - fs.rename(&foo, &foo.join("bar").join("baz"),), + fs.rename(&foo, &foo.join("bar").join("baz"),).await, Err(FsError::EntryNotFound), "renaming to a directory that has parent that doesn't exist", ); @@ -1036,7 +1038,7 @@ mod tests { assert_eq!(fs.create_dir(&bar), Ok(())); assert_eq!( - fs.rename(&foo, &bar), + fs.rename(&foo, &bar).await, Ok(()), "renaming to a directory that has parent that exists", ); @@ -1097,19 +1099,21 @@ mod tests { assert_eq!(fs.create_dir(&foo), Ok(()), "create ./foo again",); assert_eq!( - fs.rename(&bar.join("hello2.txt"), &foo.join("world2.txt")), + fs.rename(&bar.join("hello2.txt"), &foo.join("world2.txt")) + .await, Ok(()), "renaming (and moving) a file", ); assert_eq!( - fs.rename(&foo, &bar.join("baz")), + fs.rename(&foo, &bar.join("baz")).await, Ok(()), "renaming a directory", ); assert_eq!( - fs.rename(&bar.join("hello1.txt"), &bar.join("world1.txt")), + fs.rename(&bar.join("hello1.txt"), &bar.join("world1.txt")) + .await, Ok(()), "renaming a file (in the same directory)", ); @@ -1133,8 +1137,8 @@ mod tests { ); } - #[test] - fn test_metadata() { + #[tokio::test] + async fn test_metadata() { use std::thread::sleep; use std::time::Duration; @@ -1171,7 +1175,7 @@ mod tests { let bar = temp.path().join("bar"); - assert_eq!(fs.rename(&foo, &bar), Ok(())); + assert_eq!(fs.rename(&foo, &bar).await, Ok(())); let bar_metadata = fs.metadata(&bar).unwrap(); assert!(bar_metadata.ft.dir); @@ -1186,8 +1190,8 @@ mod tests { ); } - #[test] - fn test_remove_file() { + #[tokio::test] + async fn test_remove_file() { let fs = FileSystem::default(); let temp = TempDir::new().unwrap(); @@ -1221,8 +1225,8 @@ mod tests { ); } - #[test] - fn test_readdir() { + #[tokio::test] + async fn test_readdir() { let temp = TempDir::new().unwrap(); let fs = FileSystem::default(); diff --git a/lib/virtual-fs/src/lib.rs b/lib/virtual-fs/src/lib.rs index 87dfd9e44af..3a9b4d1cde8 100644 --- a/lib/virtual-fs/src/lib.rs +++ b/lib/virtual-fs/src/lib.rs @@ -2,10 +2,10 @@ #[macro_use] extern crate pretty_assertions; +use futures::future::BoxFuture; use std::any::Any; use std::ffi::OsString; use std::fmt; -use std::future::Future; use std::io; use std::ops::Deref; use std::path::{Path, PathBuf}; @@ -80,11 +80,13 @@ pub use tokio::io::{AsyncWrite, AsyncWriteExt}; pub trait ClonableVirtualFile: VirtualFile + Clone {} +pub use ops::{copy_reference, copy_reference_ext}; + pub trait FileSystem: fmt::Debug + Send + Sync + 'static + Upcastable { fn read_dir(&self, path: &Path) -> Result; fn create_dir(&self, path: &Path) -> Result<()>; fn remove_dir(&self, path: &Path) -> Result<()>; - fn rename(&self, from: &Path, to: &Path) -> Result<()>; + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>>; fn metadata(&self, path: &Path) -> Result; /// This method gets metadata without following symlinks in the path. /// Currently identical to `metadata` because symlinks aren't implemented @@ -108,6 +110,7 @@ impl dyn FileSystem + 'static { } } +#[async_trait::async_trait] impl FileSystem for D where D: Deref + std::fmt::Debug + Send + Sync + 'static, @@ -125,8 +128,8 @@ where (**self).remove_dir(path) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - (**self).rename(from, to) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { (**self).rename(from, to).await }) } fn metadata(&self, path: &Path) -> Result { @@ -314,7 +317,7 @@ impl<'a> OpenOptions<'a> { /// This trait relies on your file closing when it goes out of scope via `Drop` //#[cfg_attr(feature = "enable-serde", typetag::serde)] pub trait VirtualFile: - fmt::Debug + AsyncRead + AsyncWrite + AsyncSeek + Unpin + Upcastable + fmt::Debug + AsyncRead + AsyncWrite + AsyncSeek + Unpin + Upcastable + Send { /// the last time the file was accessed in nanoseconds as a UNIX timestamp fn last_accessed(&self) -> u64; @@ -333,7 +336,7 @@ pub trait VirtualFile: fn set_len(&mut self, new_size: u64) -> Result<()>; /// Request deletion of the file - fn unlink(&mut self) -> Result<()>; + fn unlink(&mut self) -> BoxFuture<'static, Result<()>>; /// Indicates if the file is opened or closed. This function must not block /// Defaults to a status of being constantly open @@ -351,10 +354,10 @@ pub trait VirtualFile: /// This method will copy a file from a source to this destination where /// the default is to do a straight byte copy however file system implementors /// may optimize this to do a zero copy - fn copy_reference<'a>( - &'a mut self, - mut src: Box, - ) -> Pin> + 'a>> { + fn copy_reference( + &mut self, + mut src: Box, + ) -> BoxFuture<'_, std::io::Result<()>> { Box::pin(async move { let bytes_written = tokio::io::copy(&mut src, self).await?; tracing::trace!(bytes_written, "Copying file into host filesystem",); @@ -596,6 +599,10 @@ impl DirEntry { .unwrap_or(self.path.as_os_str()) .to_owned() } + + pub fn is_white_out(&self) -> Option { + ops::is_white_out(&self.path) + } } #[allow(clippy::len_without_is_empty)] // Clippy thinks it's an iterator. diff --git a/lib/virtual-fs/src/mem_fs/file.rs b/lib/virtual-fs/src/mem_fs/file.rs index cf18889ca68..ab4c4f746b2 100644 --- a/lib/virtual-fs/src/mem_fs/file.rs +++ b/lib/virtual-fs/src/mem_fs/file.rs @@ -2,6 +2,7 @@ //! implementations. They aren't exposed to the public API. Only //! `FileHandle` can be used through the `VirtualFile` trait object. +use futures::future::BoxFuture; use tokio::io::AsyncRead; use tokio::io::{AsyncSeek, AsyncWrite}; @@ -189,8 +190,8 @@ impl VirtualFile for FileHandle { Some(Node::ReadOnlyFile { .. }) => return Err(FsError::PermissionDenied), Some(Node::ArcFile { .. }) => { drop(fs); - self.lazy_load_arc_file_mut() - .map(|file| file.set_len(new_size))??; + let file = self.lazy_load_arc_file_mut()?; + file.set_len(new_size)?; } _ => return Err(FsError::NotAFile), } @@ -198,49 +199,53 @@ impl VirtualFile for FileHandle { Ok(()) } - fn unlink(&mut self) -> Result<()> { - let (inode_of_parent, position, inode_of_file) = { - // Read lock. - let fs = self.filesystem.inner.read().map_err(|_| FsError::Lock)?; - - // The inode of the file. - let inode_of_file = self.inode; - - // Find the position of the file in the parent, and the - // inode of the parent. - let (position, inode_of_parent) = fs - .storage - .iter() - .find_map(|(inode_of_parent, node)| match node { - Node::Directory(DirectoryNode { children, .. }) => { - children.iter().enumerate().find_map(|(nth, inode)| { - if inode == &inode_of_file { - Some((nth, inode_of_parent)) - } else { - None - } - }) - } + fn unlink(&mut self) -> BoxFuture<'static, Result<()>> { + let filesystem = self.filesystem.clone(); + let inode = self.inode; + Box::pin(async move { + let (inode_of_parent, position, inode_of_file) = { + // Read lock. + let fs = filesystem.inner.read().map_err(|_| FsError::Lock)?; + + // The inode of the file. + let inode_of_file = inode; + + // Find the position of the file in the parent, and the + // inode of the parent. + let (position, inode_of_parent) = fs + .storage + .iter() + .find_map(|(inode_of_parent, node)| match node { + Node::Directory(DirectoryNode { children, .. }) => { + children.iter().enumerate().find_map(|(nth, inode)| { + if inode == &inode_of_file { + Some((nth, inode_of_parent)) + } else { + None + } + }) + } - _ => None, - }) - .ok_or(FsError::BaseNotDirectory)?; + _ => None, + }) + .ok_or(FsError::BaseNotDirectory)?; - (inode_of_parent, position, inode_of_file) - }; + (inode_of_parent, position, inode_of_file) + }; - { - // Write lock. - let mut fs = self.filesystem.inner.write().map_err(|_| FsError::Lock)?; + { + // Write lock. + let mut fs = filesystem.inner.write().map_err(|_| FsError::Lock)?; - // Remove the file from the storage. - fs.storage.remove(inode_of_file); + // Remove the file from the storage. + fs.storage.remove(inode_of_file); - // Remove the child from the parent directory. - fs.remove_child_from_node(inode_of_parent, position)?; - } + // Remove the child from the parent directory. + fs.remove_child_from_node(inode_of_parent, position)?; + } - Ok(()) + Ok(()) + }) } fn get_special_fd(&self) -> Option { @@ -276,10 +281,10 @@ impl VirtualFile for FileHandle { } } - fn copy_reference<'a>( - &'a mut self, - src: Box, - ) -> Pin> + 'a>> { + fn copy_reference( + &mut self, + src: Box, + ) -> BoxFuture<'_, std::io::Result<()>> { let inner = self.filesystem.inner.clone(); Box::pin(async move { let mut fs = inner.write().unwrap(); @@ -492,8 +497,8 @@ mod test_virtual_file { assert_eq!(file.size(), 0, "new file is empty"); } - #[test] - fn test_set_len() { + #[tokio::test] + async fn test_set_len() { let fs = FileSystem::default(); let mut file = fs @@ -507,8 +512,8 @@ mod test_virtual_file { assert_eq!(file.size(), 7, "file has a new length"); } - #[test] - fn test_unlink() { + #[tokio::test] + async fn test_unlink() { let fs = FileSystem::default(); let mut file = fs @@ -547,7 +552,7 @@ mod test_virtual_file { ); } - assert_eq!(file.unlink(), Ok(()), "unlinking the file"); + assert_eq!(file.unlink().await, Ok(()), "unlinking the file"); { let fs_inner = fs.inner.read().unwrap(); diff --git a/lib/virtual-fs/src/mem_fs/file_opener.rs b/lib/virtual-fs/src/mem_fs/file_opener.rs index 00e27cf621f..6128ce86b69 100644 --- a/lib/virtual-fs/src/mem_fs/file_opener.rs +++ b/lib/virtual-fs/src/mem_fs/file_opener.rs @@ -327,7 +327,7 @@ impl crate::FileOpener for FileSystem { path: &Path, conf: &OpenOptionsConfig, ) -> Result> { - debug!("open: path={}", path.display()); + debug!(path=%path.display(), "open"); let read = conf.read(); let mut write = conf.write(); @@ -466,7 +466,7 @@ impl crate::FileOpener for FileSystem { // The file doesn't already exist; it's OK to create it if: // 1. `create_new` is used with `write` or `append`, // 2. `create` is used with `write` or `append`. - None if (create_new || create) && (write || append) => { + None if (create_new || create) && (create_new || write || append) => { // Write lock. let mut fs = self.inner.write().map_err(|_| FsError::Lock)?; @@ -607,7 +607,7 @@ mod test_file_opener { .write(false) .create_new(true) .open(path!("/foo.txt")), - Err(FsError::PermissionDenied), + Ok(_), ), "creating a file without the `write` option", ); diff --git a/lib/virtual-fs/src/mem_fs/filesystem.rs b/lib/virtual-fs/src/mem_fs/filesystem.rs index d5a13920030..44cb1580d67 100644 --- a/lib/virtual-fs/src/mem_fs/filesystem.rs +++ b/lib/virtual-fs/src/mem_fs/filesystem.rs @@ -2,6 +2,7 @@ use super::*; use crate::{DirEntry, FileSystem as _, FileType, FsError, Metadata, OpenOptions, ReadDir, Result}; +use futures::future::BoxFuture; use slab::Slab; use std::collections::VecDeque; use std::convert::identity; @@ -417,112 +418,114 @@ impl crate::FileSystem for FileSystem { Ok(()) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - let name_of_to; + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { + let name_of_to; - let ( - (position_of_from, inode, inode_of_from_parent), - (inode_of_to_parent, name_of_to), - inode_dest, - ) = { - // Read lock. - let fs = self.inner.read().map_err(|_| FsError::Lock)?; - - let from = fs.canonicalize_without_inode(from)?; - let to = fs.canonicalize_without_inode(to)?; - - // Check the paths have parents. - let parent_of_from = from.parent().ok_or(FsError::BaseNotDirectory)?; - let parent_of_to = to.parent().ok_or(FsError::BaseNotDirectory)?; - - // Check the names. - let name_of_from = from - .file_name() - .ok_or(FsError::InvalidInput)? - .to_os_string(); - name_of_to = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string(); + let ( + (position_of_from, inode, inode_of_from_parent), + (inode_of_to_parent, name_of_to), + inode_dest, + ) = { + // Read lock. + let fs = self.inner.read().map_err(|_| FsError::Lock)?; + + let from = fs.canonicalize_without_inode(from)?; + let to = fs.canonicalize_without_inode(to)?; + + // Check the paths have parents. + let parent_of_from = from.parent().ok_or(FsError::BaseNotDirectory)?; + let parent_of_to = to.parent().ok_or(FsError::BaseNotDirectory)?; + + // Check the names. + let name_of_from = from + .file_name() + .ok_or(FsError::InvalidInput)? + .to_os_string(); + name_of_to = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string(); + + // Find the parent inodes. + let inode_of_from_parent = match fs.inode_of_parent(parent_of_from)? { + InodeResolution::Found(a) => a, + InodeResolution::Redirect(..) => { + return Err(FsError::InvalidInput); + } + }; + let inode_of_to_parent = match fs.inode_of_parent(parent_of_to)? { + InodeResolution::Found(a) => a, + InodeResolution::Redirect(..) => { + return Err(FsError::InvalidInput); + } + }; - // Find the parent inodes. - let inode_of_from_parent = match fs.inode_of_parent(parent_of_from)? { - InodeResolution::Found(a) => a, - InodeResolution::Redirect(..) => { - return Err(FsError::InvalidInput); - } + // Find the inode of the dest file if it exists + let maybe_position_and_inode_of_file = + fs.as_parent_get_position_and_inode_of_file(inode_of_to_parent, &name_of_to)?; + + // Get the child indexes to update in the parent nodes, in + // addition to the inode of the directory to update. + let (position_of_from, inode) = fs + .as_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)? + .ok_or(FsError::EntryNotFound)?; + + ( + (position_of_from, inode, inode_of_from_parent), + (inode_of_to_parent, name_of_to), + maybe_position_and_inode_of_file, + ) }; - let inode_of_to_parent = match fs.inode_of_parent(parent_of_to)? { + + let inode = match inode { InodeResolution::Found(a) => a, InodeResolution::Redirect(..) => { return Err(FsError::InvalidInput); } }; - // Find the inode of the dest file if it exists - let maybe_position_and_inode_of_file = - fs.as_parent_get_position_and_inode_of_file(inode_of_to_parent, &name_of_to)?; - - // Get the child indexes to update in the parent nodes, in - // addition to the inode of the directory to update. - let (position_of_from, inode) = fs - .as_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)? - .ok_or(FsError::EntryNotFound)?; - - ( - (position_of_from, inode, inode_of_from_parent), - (inode_of_to_parent, name_of_to), - maybe_position_and_inode_of_file, - ) - }; - - let inode = match inode { - InodeResolution::Found(a) => a, - InodeResolution::Redirect(..) => { - return Err(FsError::InvalidInput); - } - }; - - { - // Write lock. - let mut fs = self.inner.write().map_err(|_| FsError::Lock)?; - - if let Some((position, inode_of_file)) = inode_dest { - // Remove the file from the storage. - match inode_of_file { - InodeResolution::Found(inode_of_file) => { - fs.storage.remove(inode_of_file); - } - InodeResolution::Redirect(..) => { - return Err(FsError::InvalidInput); + { + // Write lock. + let mut fs = self.inner.write().map_err(|_| FsError::Lock)?; + + if let Some((position, inode_of_file)) = inode_dest { + // Remove the file from the storage. + match inode_of_file { + InodeResolution::Found(inode_of_file) => { + fs.storage.remove(inode_of_file); + } + InodeResolution::Redirect(..) => { + return Err(FsError::InvalidInput); + } } - } - fs.remove_child_from_node(inode_of_to_parent, position)?; - } + fs.remove_child_from_node(inode_of_to_parent, position)?; + } - // Update the file name, and update the modified time. - fs.update_node_name(inode, name_of_to)?; + // Update the file name, and update the modified time. + fs.update_node_name(inode, name_of_to)?; - // The parents are different. Let's update them. - if inode_of_from_parent != inode_of_to_parent { - // Remove the file from its parent, and update the - // modified time. - fs.remove_child_from_node(inode_of_from_parent, position_of_from)?; + // The parents are different. Let's update them. + if inode_of_from_parent != inode_of_to_parent { + // Remove the file from its parent, and update the + // modified time. + fs.remove_child_from_node(inode_of_from_parent, position_of_from)?; - // Add the file to its new parent, and update the modified - // time. - fs.add_child_to_node(inode_of_to_parent, inode)?; - } - // Otherwise, we need to at least update the modified time of the parent. - else { - let mut inode = fs.storage.get_mut(inode_of_from_parent); - match inode.as_mut() { - Some(Node::Directory(node)) => node.metadata.modified = time(), - Some(Node::ArcDirectory(node)) => node.metadata.modified = time(), - _ => return Err(FsError::UnknownError), + // Add the file to its new parent, and update the modified + // time. + fs.add_child_to_node(inode_of_to_parent, inode)?; + } + // Otherwise, we need to at least update the modified time of the parent. + else { + let mut inode = fs.storage.get_mut(inode_of_from_parent); + match inode.as_mut() { + Some(Node::Directory(node)) => node.metadata.modified = time(), + Some(Node::ArcDirectory(node)) => node.metadata.modified = time(), + _ => return Err(FsError::UnknownError), + } } } - } - Ok(()) + Ok(()) + }) } fn metadata(&self, path: &Path) -> Result { @@ -1039,7 +1042,7 @@ mod test_filesystem { use tokio::io::AsyncReadExt; - use crate::{mem_fs::*, ops, DirEntry, FileOpener, FileSystem as FS, FileType, FsError}; + use crate::{mem_fs::*, ops, DirEntry, FileSystem as FS, FileType, FsError}; macro_rules! path { ($path:expr) => { @@ -1225,17 +1228,17 @@ mod test_filesystem { } } - #[test] - fn test_rename() { + #[tokio::test] + async fn test_rename() { let fs = FileSystem::default(); assert_eq!( - fs.rename(path!("/"), path!("/bar")), + fs.rename(path!("/"), path!("/bar")).await, Err(FsError::BaseNotDirectory), "renaming a directory that has no parent", ); assert_eq!( - fs.rename(path!("/foo"), path!("/")), + fs.rename(path!("/foo"), path!("/")).await, Err(FsError::BaseNotDirectory), "renaming to a directory that has no parent", ); @@ -1244,7 +1247,7 @@ mod test_filesystem { assert_eq!(fs.create_dir(path!("/foo/qux")), Ok(())); assert_eq!( - fs.rename(path!("/foo"), path!("/bar/baz")), + fs.rename(path!("/foo"), path!("/bar/baz")).await, Err(FsError::EntryNotFound), "renaming to a directory that has parent that doesn't exist", ); @@ -1349,19 +1352,21 @@ mod test_filesystem { } assert_eq!( - fs.rename(path!("/bar/hello2.txt"), path!("/foo/world2.txt")), + fs.rename(path!("/bar/hello2.txt"), path!("/foo/world2.txt")) + .await, Ok(()), "renaming (and moving) a file", ); assert_eq!( - fs.rename(path!("/foo"), path!("/bar/baz")), + fs.rename(path!("/foo"), path!("/bar/baz")).await, Ok(()), "renaming a directory", ); assert_eq!( - fs.rename(path!("/bar/hello1.txt"), path!("/bar/world1.txt")), + fs.rename(path!("/bar/hello1.txt"), path!("/bar/world1.txt")) + .await, Ok(()), "renaming a file (in the same directory)", ); @@ -1449,8 +1454,8 @@ mod test_filesystem { } } - #[test] - fn test_metadata() { + #[tokio::test] + async fn test_metadata() { use std::thread::sleep; use std::time::Duration; @@ -1487,7 +1492,7 @@ mod test_filesystem { sleep(Duration::from_secs(3)); - assert_eq!(fs.rename(path!("/foo"), path!("/bar")), Ok(())); + assert_eq!(fs.rename(path!("/foo"), path!("/bar")).await, Ok(())); assert!( matches!( diff --git a/lib/virtual-fs/src/mem_fs/stdio.rs b/lib/virtual-fs/src/mem_fs/stdio.rs index 525e9fd8230..ccab27aad4c 100644 --- a/lib/virtual-fs/src/mem_fs/stdio.rs +++ b/lib/virtual-fs/src/mem_fs/stdio.rs @@ -41,12 +41,14 @@ macro_rules! impl_virtualfile_on_std_streams { 0 } - fn set_len(&mut self, _new_size: u64) -> Result<()> { + fn set_len(& mut self, _new_size: u64) -> Result<()> { Err(FsError::PermissionDenied) } - fn unlink(&mut self) -> Result<()> { - Ok(()) + fn unlink(&mut self) -> futures::future::BoxFuture<'static, Result<()>> { + Box::pin(async { + Ok(()) + }) } fn poll_read_ready(self: std::pin::Pin<&mut Self>, _cx: &mut std::task::Context<'_>) -> std::task::Poll> { diff --git a/lib/virtual-fs/src/null_file.rs b/lib/virtual-fs/src/null_file.rs index b211314d482..2ddb156a6bb 100644 --- a/lib/virtual-fs/src/null_file.rs +++ b/lib/virtual-fs/src/null_file.rs @@ -5,6 +5,7 @@ use std::io::{self, *}; use std::pin::Pin; use std::task::{Context, Poll}; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use crate::{ClonableVirtualFile, VirtualFile}; @@ -73,8 +74,8 @@ impl VirtualFile for NullFile { fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { Poll::Ready(Ok(8192)) diff --git a/lib/virtual-fs/src/ops.rs b/lib/virtual-fs/src/ops.rs index 2c124d9b8e7..88c9c066808 100644 --- a/lib/virtual-fs/src/ops.rs +++ b/lib/virtual-fs/src/ops.rs @@ -1,8 +1,12 @@ //! Common [`FileSystem`] operations. #![allow(dead_code)] // Most of these helpers are used during testing -use std::{collections::VecDeque, path::Path}; +use std::{ + collections::VecDeque, + path::{Path, PathBuf}, +}; +use futures::future::BoxFuture; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use crate::{DirEntry, FileSystem, FsError}; @@ -61,6 +65,102 @@ where fs.create_dir(path) } +static WHITEOUT_PREFIX: &str = ".wh."; + +/// Creates a white out file which hides it from secondary file systems +pub fn create_white_out(fs: &F, path: impl AsRef) -> Result<(), FsError> +where + F: FileSystem + ?Sized, +{ + if let Some(filename) = path.as_ref().file_name() { + let mut path = path.as_ref().to_owned(); + path.set_file_name(format!("{}{}", WHITEOUT_PREFIX, filename.to_string_lossy())); + + if let Some(parent) = path.parent() { + create_dir_all(fs, parent).ok(); + } + + fs.new_open_options() + .create_new(true) + .truncate(true) + .write(true) + .open(path)?; + Ok(()) + } else { + Err(FsError::EntryNotFound) + } +} + +/// Removes a white out file from the primary +pub fn remove_white_out(fs: &F, path: impl AsRef) +where + F: FileSystem + ?Sized, +{ + if let Some(filename) = path.as_ref().file_name() { + let mut path = path.as_ref().to_owned(); + path.set_file_name(format!("{}{}", WHITEOUT_PREFIX, filename.to_string_lossy())); + fs.remove_file(&path).ok(); + } +} + +/// Returns true if the path has been hidden by a whiteout file +pub fn has_white_out(fs: &F, path: impl AsRef) -> bool +where + F: FileSystem + ?Sized, +{ + if let Some(filename) = path.as_ref().file_name() { + let mut path = path.as_ref().to_owned(); + path.set_file_name(format!("{}{}", WHITEOUT_PREFIX, filename.to_string_lossy())); + fs.metadata(&path).is_ok() + } else { + false + } +} + +/// Returns true if the path is a whiteout file +pub fn is_white_out(path: impl AsRef) -> Option { + if let Some(filename) = path.as_ref().file_name() { + if let Some(filename) = filename.to_string_lossy().strip_prefix(WHITEOUT_PREFIX) { + let mut path = path.as_ref().to_owned(); + path.set_file_name(filename); + return Some(path); + } + } + None +} + +/// Copies the reference of a file from one file system to another +pub fn copy_reference<'a>( + source: &'a (impl FileSystem + ?Sized), + destination: &'a (impl FileSystem + ?Sized), + path: &'a Path, +) -> BoxFuture<'a, Result<(), std::io::Error>> { + Box::pin(async { copy_reference_ext(source, destination, path, path).await }) +} + +/// Copies the reference of a file from one file system to another +pub fn copy_reference_ext<'a>( + source: &'a (impl FileSystem + ?Sized), + destination: &'a (impl FileSystem + ?Sized), + from: &Path, + to: &Path, +) -> BoxFuture<'a, Result<(), std::io::Error>> { + let from = from.to_owned(); + let to = to.to_owned(); + Box::pin(async move { + let src = source.new_open_options().read(true).open(from)?; + let mut dst = destination + .new_open_options() + .create(true) + .write(true) + .truncate(true) + .open(to)?; + + dst.copy_reference(src).await?; + Ok(()) + }) +} + /// Asynchronously write some bytes to a file. /// /// This is analogous to [`std::fs::write()`]. diff --git a/lib/virtual-fs/src/overlay_fs.rs b/lib/virtual-fs/src/overlay_fs.rs index ed5e73f54c7..2b10e29dafe 100644 --- a/lib/virtual-fs/src/overlay_fs.rs +++ b/lib/virtual-fs/src/overlay_fs.rs @@ -1,11 +1,16 @@ use std::{ + collections::HashSet, fmt::Debug, + io::{self, SeekFrom}, path::{Path, PathBuf}, pin::Pin, - task::Poll, + sync::Arc, + task::{Context, Poll}, }; -use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; +use futures::future::BoxFuture; +use replace_with::replace_with_or_abort; +use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite, ReadBuf}; use crate::{ ops, FileOpener, FileSystem, FileSystems, FsError, Metadata, OpenOptions, OpenOptionsConfig, @@ -51,20 +56,20 @@ use crate::{ /// A more complex example is #[derive(Clone, PartialEq, Eq)] pub struct OverlayFileSystem { - primary: P, + primary: Arc

, secondaries: S, } impl OverlayFileSystem where - P: FileSystem + 'static, + P: FileSystem + Send + Sync + 'static, S: for<'a> FileSystems<'a> + Send + Sync + 'static, { /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and a /// chain of secondary [`FileSystems`]. pub fn new(primary: P, secondaries: S) -> Self { OverlayFileSystem { - primary, + primary: Arc::new(primary), secondaries, } } @@ -74,11 +79,6 @@ where &self.primary } - /// Get a mutable reference to the primary filesystem. - pub fn primary_mut(&mut self) -> &mut P { - &mut self.primary - } - /// Get a reference to the secondary filesystems. pub fn secondaries(&self) -> &S { &self.secondaries @@ -89,12 +89,6 @@ where &mut self.secondaries } - /// Consume the [`OverlayFileSystem`], returning the underlying primary and - /// secondary filesystems. - pub fn into_inner(self) -> (P, S) { - (self.primary, self.secondaries) - } - fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { for fs in self.secondaries.filesystems() { if ops::exists(fs, path) { @@ -108,14 +102,17 @@ where impl FileSystem for OverlayFileSystem where - P: FileSystem + 'static, + P: FileSystem + Send + 'static, S: for<'a> FileSystems<'a> + Send + Sync + 'static, + for<'a> <>::Iter as IntoIterator>::IntoIter: Send, { + #[tracing::instrument(level = "debug", skip_all, fields(path=%path.display()))] fn read_dir(&self, path: &Path) -> Result { let mut entries = Vec::new(); let mut had_at_least_one_success = false; + let mut white_outs = HashSet::new(); - let filesystems = std::iter::once(&self.primary as &dyn FileSystem) + let filesystems = std::iter::once(&self.primary as &(dyn FileSystem + Send)) .into_iter() .chain(self.secondaries().filesystems()); @@ -123,7 +120,27 @@ where match fs.read_dir(path) { Ok(r) => { for entry in r { - entries.push(entry?); + let entry = entry?; + + // White out entries block any later entries in the secondaries + // unless the entry has comes before the white out, thus the order + // that the file systems are parsed is important to this logic. + if let Some(path) = entry.is_white_out() { + tracing::trace!( + path=%path.display(), + "Found whiteout file", + ); + white_outs.insert(path); + continue; + } else if white_outs.contains(&entry.path) { + tracing::trace!( + path=%path.display(), + "Skipping path because a whiteout exists", + ); + continue; + } + + entries.push(entry); } had_at_least_one_success = true; } @@ -147,6 +164,25 @@ where } fn create_dir(&self, path: &Path) -> Result<(), FsError> { + // You can not create directories that use the whiteout prefix + if ops::is_white_out(path).is_some() { + return Err(FsError::InvalidInput); + } + + // It could be the case that the directory was earlier hidden in the secondaries + // by a whiteout file, hence we need to make sure those are cleared out. + ops::remove_white_out(self.primary.as_ref(), path); + + // Make sure the parent tree is in place on the primary, this is to cover the + // scenario where the secondaries has a parent structure that is not yet in the + // primary and the primary needs it to create a sub-directory + if let Some(parent) = path.parent() { + if self.read_dir(parent).is_ok() { + ops::create_dir_all(&self.primary, parent).ok(); + } + } + + // Create the directory in the primary match self.primary.create_dir(path) { Err(e) if should_continue(e) => {} other => return other, @@ -156,30 +192,127 @@ where } fn remove_dir(&self, path: &Path) -> Result<(), FsError> { + // Whiteout files can not be removed, instead the original directory + // must be removed or recreated. + if ops::is_white_out(path).is_some() { + tracing::trace!( + path=%path.display(), + "Unable to remove a whited out directory", + ); + return Err(FsError::EntryNotFound); + } + + // If the directory is contained in a secondary file system then we need to create a + // whiteout file so that it is suppressed and is no longer returned in `readdir` calls. + + let had_at_least_one_success = self.secondaries.filesystems().into_iter().any(|fs| { + fs.read_dir(path).is_ok() && ops::create_white_out(&self.primary, path).is_ok() + }); + + // Attempt to remove it from the primary, if this succeeds then we may have also + // added the whiteout file in the earlier step, but are required in this case to + // properly delete the directory. match self.primary.remove_dir(path) { Err(e) if should_continue(e) => {} other => return other, } + if had_at_least_one_success { + return Ok(()); + } self.permission_error_or_not_found(path) } - fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - match self.primary.rename(from, to) { - Err(e) if should_continue(e) => {} - other => return other, - } + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<(), FsError>> { + let from = from.to_owned(); + let to = to.to_owned(); + Box::pin(async move { + // Whiteout files can not be renamed + if ops::is_white_out(&from).is_some() { + tracing::trace!( + from=%from.display(), + to=%to.display(), + "Attempting to rename a file that was whited out" + ); + return Err(FsError::EntryNotFound); + } + // You can not rename a file into a whiteout file + if ops::is_white_out(&to).is_some() { + tracing::trace!( + from=%from.display(), + to=%to.display(), + "Attempting to rename a file into a whiteout file" + ); + return Err(FsError::InvalidInput); + } + + // We attempt to rename the file or directory in the primary + // if this succeeds then we also need to ensure the white out + // files are created where we need them, so we do not immediately + // return until that is done + let mut had_at_least_one_success = false; + match self.primary.rename(&from, &to).await { + Err(e) if should_continue(e) => {} + Ok(()) => { + had_at_least_one_success = true; + } + other => return other, + } + + // If we have not yet renamed the file it may still reside in + // the secondaries, in which case we need to copy it to the + // primary rather than rename it + if !had_at_least_one_success { + for fs in self.secondaries.filesystems() { + if fs.metadata(&from).is_ok() { + ops::copy_reference_ext(fs, &self.primary, &from, &to).await?; + had_at_least_one_success = true; + break; + } + } + } + + // If the rename operation was a success then we need to update any + // whiteout files on the primary before we return success. + if had_at_least_one_success { + for fs in self.secondaries.filesystems() { + if fs.metadata(&from).is_ok() { + tracing::trace!( + path=%from.display(), + "Creating a whiteout for the file that was renamed", + ); + ops::create_white_out(&self.primary, &from).ok(); + break; + } + } + ops::remove_white_out(&self.primary, &to); + return Ok(()); + } - self.permission_error_or_not_found(from) + // Otherwise we are in a failure scenario + self.permission_error_or_not_found(&from) + }) } fn metadata(&self, path: &Path) -> Result { + // Whiteout files can not be read, they are just markers + if ops::is_white_out(path).is_some() { + return Err(FsError::EntryNotFound); + } + + // Check if the file is in the primary match self.primary.metadata(path) { Ok(meta) => return Ok(meta), Err(e) if should_continue(e) => {} Err(e) => return Err(e), } + // There might be a whiteout, search for this + if ops::has_white_out(&self.primary, path) { + return Err(FsError::EntryNotFound); + } + + // Otherwise scan the secondaries for fs in self.secondaries.filesystems() { match fs.metadata(path) { Err(e) if should_continue(e) => continue, @@ -191,11 +324,27 @@ where } fn remove_file(&self, path: &Path) -> Result<(), FsError> { + // It is not possible to delete whiteout files directly, instead + // one must delete the original file + if ops::is_white_out(path).is_some() { + return Err(FsError::InvalidInput); + } + + // If the file is contained in a secondary then then we need to create a + // whiteout file so that it is suppressed. + let had_at_least_one_success = self.secondaries.filesystems().into_iter().any(|fs| { + fs.metadata(path).is_ok() && ops::create_white_out(&self.primary, path).is_ok() + }); + + // Attempt to remove it from the primary match self.primary.remove_file(path) { Err(e) if should_continue(e) => {} other => return other, } + if had_at_least_one_success { + return Ok(()); + } self.permission_error_or_not_found(path) } @@ -206,229 +355,650 @@ where impl FileOpener for OverlayFileSystem where - P: FileSystem, + P: FileSystem + Send + 'static, S: for<'a> FileSystems<'a> + Send + Sync + 'static, + for<'a> <>::Iter as IntoIterator>::IntoIter: Send, { fn open( &self, path: &Path, conf: &OpenOptionsConfig, ) -> Result, FsError> { - match self - .primary - .new_open_options() - .options(conf.clone()) - .open(path) + // Whiteout files can not be read, they are just markers + if ops::is_white_out(path).is_some() { + tracing::trace!( + path=%path.display(), + options=?conf, + "Whiteout files can't be opened", + ); + return Err(FsError::InvalidInput); + } + + // Check if the file is in the primary (without actually creating it) as + // when the file is in the primary it takes preference over any of file { - Err(e) if should_continue(e) => {} - other => return other, + let mut conf = conf.clone(); + conf.create = false; + conf.create_new = false; + match self.primary.new_open_options().options(conf).open(path) { + Err(e) if should_continue(e) => {} + other => return other, + } } - if (conf.create || conf.create_new) && !ops::exists(self, path) { + // In the scenario that we are creating the file then there is + // special handling that will ensure its setup correctly + if conf.create_new { + // When the secondary has the directory structure but the primary + // does not then we need to make sure we create all the structure + // in the primary if let Some(parent) = path.parent() { - let parent_exists_on_secondary_fs = self - .secondaries - .filesystems() - .into_iter() - .any(|fs| ops::is_dir(fs, parent)); - if parent_exists_on_secondary_fs { - // We fall into the special case where you can create a file - // that looks like it is inside a secondary filesystem folder, - // but actually it gets created on the host + if ops::exists(self, parent) { + // We create the directory structure on the primary so that + // the new file can be created, this will make it override + // whatever is in the primary ops::create_dir_all(&self.primary, parent)?; - return self - .primary - .new_open_options() - .options(conf.clone()) - .open(path); } else { return Err(FsError::EntryNotFound); } } + + // Remove any whiteout + ops::remove_white_out(&self.primary, path); + + // Create the file in the primary + return self + .primary + .new_open_options() + .options(conf.clone()) + .open(path); } - if opening_would_require_mutations(&self.secondaries, path, conf) { - // HACK: we should return Err(FsError::PermissionDenied) here - return open_readonly_file_hack(path, conf, &self.secondaries); + // There might be a whiteout, search for this and if its found then + // we are done as the secondary file or directory has been earlier + // deleted via a white out (when the create flag is set then + // the white out marker is ignored) + if !conf.create && ops::has_white_out(&self.primary, path) { + tracing::trace!( + path=%path.display(), + "The file has been whited out", + ); + return Err(FsError::EntryNotFound); } - for fs in self.secondaries.filesystems() { - match fs.new_open_options().options(conf.clone()).open(path) { - Err(e) if should_continue(e) => continue, - other => return other, + // Determine if a mutation will be possible with the opened file + let require_mutations = conf.append || conf.write || conf.create_new | conf.truncate; + + // If the file is on a secondary then we should open it + if !ops::has_white_out(&self.primary, path) { + for fs in self.secondaries.filesystems() { + let mut sub_conf = conf.clone(); + sub_conf.create = false; + sub_conf.create_new = false; + sub_conf.append = false; + sub_conf.truncate = false; + match fs.new_open_options().options(sub_conf.clone()).open(path) { + Err(e) if should_continue(e) => continue, + Ok(file) if require_mutations => { + // If the file was opened with the ability to mutate then we need + // to return a copy on write emulation so that the file can be + // copied from the secondary to the primary in the scenario that + // it is edited + return open_copy_on_write(path, conf, &self.primary, file); + } + other => return other, + } + } + } + + // If we are creating the file then do so + if conf.create { + // Create the parent structure and remove any whiteouts + if let Some(parent) = path.parent() { + if ops::exists(self, parent) { + ops::create_dir_all(&self.primary, parent)?; + } } + ops::remove_white_out(&self.primary, path); + + // Create the file in the primary + return self + .primary + .new_open_options() + .options(conf.clone()) + .open(path); } + // The file does not exist anywhere Err(FsError::EntryNotFound) } } -/// HACK(Michael-F-Bryan): In theory, you shouldn't be able to open a file in -/// one of the [`OverlayFileSystem`]'s secondaries in write mode because the -/// filesystem is meant to be readonly. However, Python does things like -/// `open("./lib/python3.6/io.py", "rw")` when importing its standard library -/// and we want Python to work, so we'll defer the [`FsError::PermissionDenied`] -/// error until the first write operation. -/// -/// We shouldn't need to do this because opening a secondary fs's file in write -/// mode goes against the "read-write primary, readonly secondaries" goal. -fn open_readonly_file_hack( +fn open_copy_on_write

( path: &Path, conf: &OpenOptionsConfig, - secondaries: &S, + primary: &Arc

, + file: Box, ) -> Result, FsError> where - S: for<'a> FileSystems<'a> + Send + Sync + 'static, + P: FileSystem, { - #[derive(Debug)] - struct ReadOnlyFile { + struct CopyOnWriteFile

{ path: PathBuf, - inner: Box, + primary: Arc

, + state: CowState, + readable: bool, + append: bool, + new_size: Option, + } + enum CowState { + // The original file is still open and can be accessed for all + // read operations + ReadOnly(Box), + // The copy has started but first we need to get the cursor + // position within the source file so that it can be restored + SeekingGet(Box), + // Now we have the original starting cursor location we need + // to move the position of the read to the start of the source + // file + SeekingSet { + original_offset: u64, + src: Box, + }, + // We are now copying the data in parts held in the buffer piece + // by piece until the original file is completely copied + Copying { + original_offset: u64, + buf: Vec, + buf_pos: usize, + dst: Box, + src: Box, + }, + // After copying the file we need to seek the position back + // to its original location on the newly copied file + SeekingRestore { + dst: Box, + }, + // We have copied the file and can use all the normal operations + Copied(Box), + // An error occurred during the copy operation and we are now in a + // failed state, after the error is consumed it will reset back + // to the original file + Error { + err: io::Error, + src: Box, + }, + } + impl CowState { + fn as_ref(&self) -> &(dyn VirtualFile + Send + Sync) { + match self { + Self::ReadOnly(inner) => inner.as_ref(), + Self::SeekingGet(inner) => inner.as_ref(), + Self::SeekingSet { src, .. } => src.as_ref(), + Self::Copying { src, .. } => src.as_ref(), + Self::SeekingRestore { dst, .. } => dst.as_ref(), + Self::Copied(inner) => inner.as_ref(), + Self::Error { src, .. } => src.as_ref(), + } + } + fn as_mut(&mut self) -> &mut (dyn VirtualFile + Send + Sync) { + match self { + Self::ReadOnly(inner) => inner.as_mut(), + Self::SeekingGet(inner) => inner.as_mut(), + Self::SeekingSet { src, .. } => src.as_mut(), + Self::Copying { src, .. } => src.as_mut(), + Self::SeekingRestore { dst, .. } => dst.as_mut(), + Self::Copied(inner) => inner.as_mut(), + Self::Error { src, .. } => src.as_mut(), + } + } } - impl VirtualFile for ReadOnlyFile { + impl

CopyOnWriteFile

+ where + P: FileSystem + 'static, + { + fn poll_copy_progress(&mut self, cx: &mut Context) -> Poll> { + // Enter a loop until we go pending + let mut again = true; + while again { + again = false; + + // The state machine is updated during the poll operation + replace_with_or_abort(&mut self.state, |state| match state { + // We record the current position of the file so that it can be + // restored after the copy-on-write is finished + CowState::SeekingGet(mut src) => { + match Pin::new(src.as_mut()).poll_complete(cx) { + Poll::Ready(Ok(offset)) => { + if let Err(err) = + Pin::new(src.as_mut()).start_seek(SeekFrom::Start(0)) + { + return CowState::Error { err, src }; + } + again = true; + CowState::SeekingSet { + original_offset: offset, + src, + } + } + Poll::Ready(Err(err)) => CowState::Error { err, src }, + Poll::Pending => CowState::SeekingGet(src), + } + } + + // We complete the seek operation to the start of the source file + CowState::SeekingSet { + original_offset, + mut src, + } => { + match Pin::new(src.as_mut()).poll_complete(cx).map_ok(|_| ()) { + Poll::Ready(Ok(())) => { + // Remove the whiteout, create the parent structure and open + // the new file on the primary + if let Some(parent) = self.path.parent() { + ops::create_dir_all(&self.primary, parent).ok(); + } + let mut had_white_out = false; + if ops::has_white_out(&self.primary, &self.path) { + ops::remove_white_out(&self.primary, &self.path); + had_white_out = true; + } + let dst = self + .primary + .new_open_options() + .create(true) + .read(self.readable) + .write(true) + .truncate(true) + .open(&self.path); + match dst { + Ok(dst) if had_white_out => { + again = true; + CowState::Copied(dst) + } + Ok(dst) => { + again = true; + CowState::Copying { + original_offset, + buf: Vec::new(), + buf_pos: 0, + src, + dst, + } + } + Err(err) => CowState::Error { + err: err.into(), + src, + }, + } + } + Poll::Ready(Err(err)) => CowState::Error { err, src }, + Poll::Pending => CowState::SeekingSet { + original_offset, + src, + }, + } + } + // We are now copying all the data on blocks + CowState::Copying { + mut src, + mut dst, + mut buf, + mut buf_pos, + original_offset, + } => { + loop { + // We are either copying more data from the source + // or we are copying the data to the destination + if buf_pos < buf.len() { + let dst_pinned = Pin::new(dst.as_mut()); + match dst_pinned.poll_write(cx, &buf[buf_pos..]) { + Poll::Ready(Ok(0)) => {} + Poll::Ready(Ok(amt)) => { + buf_pos += amt; + continue; + } + Poll::Ready(Err(err)) => { + return CowState::Error { err, src }; + } + Poll::Pending => {} + } + } else { + buf.resize_with(8192, || 0); + buf_pos = 8192; + let mut read_buf = ReadBuf::new(&mut buf); + match Pin::new(src.as_mut()).poll_read(cx, &mut read_buf) { + Poll::Ready(Ok(())) if read_buf.filled().is_empty() => { + again = true; + + if self.append { + // When we append then we leave the cursor at the + // end of the file + return CowState::Copied(dst); + } else { + // No more data exists to be read so we now move on to + // restoring the cursor back to the original position + if let Err(err) = Pin::new(dst.as_mut()) + .start_seek(SeekFrom::Start(original_offset)) + { + return CowState::Error { err, src }; + } + return CowState::SeekingRestore { dst }; + } + } + Poll::Ready(Ok(())) => { + // There is more data to be processed + let new_len = read_buf.filled().len(); + unsafe { buf.set_len(new_len) }; + buf_pos = 0; + continue; + } + Poll::Ready(Err(err)) => return CowState::Error { err, src }, + Poll::Pending => {} + } + } + return CowState::Copying { + original_offset, + buf, + buf_pos, + src, + dst, + }; + } + } + // Now once the the restoration of the seek position completes we set the copied state + CowState::SeekingRestore { mut dst } => { + match Pin::new(dst.as_mut()).poll_complete(cx) { + Poll::Ready(_) => { + // If we have changed the length then set it + if let Some(new_size) = self.new_size.take() { + dst.set_len(new_size).ok(); + } + CowState::Copied(dst) + } + Poll::Pending => CowState::SeekingRestore { dst }, + } + } + s => s, + }); + } + + // Determine what response to give based off the state, when an error occurs + // this will be returned and the copy-on-write will be reset + let mut ret = Poll::Pending; + replace_with_or_abort(&mut self.state, |state| match state { + CowState::ReadOnly(src) => { + ret = Poll::Ready(Ok(())); + CowState::ReadOnly(src) + } + CowState::Copied(src) => { + ret = Poll::Ready(Ok(())); + CowState::Copied(src) + } + CowState::Error { err, src } => { + ret = Poll::Ready(Err(err)); + CowState::ReadOnly(src) + } + state => { + ret = Poll::Pending; + state + } + }); + ret + } + + fn poll_copy_start_and_progress(&mut self, cx: &mut Context) -> Poll> { + replace_with_or_abort(&mut self.state, |state| match state { + CowState::ReadOnly(inner) => CowState::SeekingGet(inner), + state => state, + }); + self.poll_copy_progress(cx) + } + } + + impl

Debug for CopyOnWriteFile

+ where + P: FileSystem + 'static, + { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("CopyOnWriteFile").finish() + } + } + + impl

VirtualFile for CopyOnWriteFile

+ where + P: FileSystem + 'static, + { fn last_accessed(&self) -> u64 { - self.inner.last_accessed() + self.state.as_ref().last_accessed() } fn last_modified(&self) -> u64 { - self.inner.last_modified() + self.state.as_ref().last_modified() } fn created_time(&self) -> u64 { - self.inner.created_time() + self.state.as_ref().created_time() } fn size(&self) -> u64 { - self.inner.size() + self.state.as_ref().size() } fn set_len(&mut self, new_size: u64) -> crate::Result<()> { - self.inner.set_len(new_size) + self.new_size = Some(new_size); + replace_with_or_abort(&mut self.state, |state| match state { + CowState::Copied(mut file) => { + file.set_len(new_size).ok(); + CowState::Copied(file) + } + state => { + // in the scenario where the length is set but the file is not + // polled then we need to make sure we create a file properly + if let Some(parent) = self.path.parent() { + ops::create_dir_all(&self.primary, parent).ok(); + } + let dst = self + .primary + .new_open_options() + .create(true) + .write(true) + .open(&self.path); + if let Ok(mut file) = dst { + file.set_len(new_size).ok(); + } + state + } + }); + Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Err(FsError::PermissionDenied) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + let primary = self.primary.clone(); + let path = self.path.clone(); + Box::pin(async move { + // Create the whiteout file in the primary + let mut had_at_least_one_success = false; + if ops::create_white_out(&primary, &path).is_ok() { + had_at_least_one_success = true; + } + + // Attempt to remove it from the primary first + match primary.remove_file(&path) { + Err(e) if should_continue(e) => {} + other => return other, + } + + if had_at_least_one_success { + return Ok(()); + } + Err(FsError::PermissionDenied) + }) } fn poll_read_ready( mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>, ) -> Poll> { - Pin::new(&mut *self.inner).poll_read_ready(cx) + match self.poll_copy_progress(cx) { + Poll::Ready(Ok(())) => {} + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Pending => return Poll::Pending, + } + Pin::new(self.state.as_mut()).poll_read_ready(cx) } fn poll_write_ready( mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>, ) -> Poll> { - Pin::new(&mut *self.inner).poll_write_ready(cx) + match self.poll_copy_progress(cx) { + Poll::Ready(Ok(())) => {} + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Pending => return Poll::Pending, + } + Pin::new(self.state.as_mut()).poll_write_ready(cx) } } - impl AsyncWrite for ReadOnlyFile { + impl

AsyncWrite for CopyOnWriteFile

+ where + P: FileSystem + 'static, + { fn poll_write( - self: Pin<&mut Self>, - _cx: &mut std::task::Context<'_>, - _buf: &[u8], + mut self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + buf: &[u8], ) -> Poll> { - tracing::warn!( - path=%self.path.display(), - "Attempting to write to a readonly file", - ); - Poll::Ready(Err(std::io::ErrorKind::PermissionDenied.into())) + match self.poll_copy_start_and_progress(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Ready(Ok(())) => {} + } + Pin::new(self.state.as_mut()).poll_write(cx, buf) + } + + fn poll_write_vectored( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + bufs: &[io::IoSlice<'_>], + ) -> Poll> { + match self.poll_copy_start_and_progress(cx) { + Poll::Pending => return Poll::Pending, + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Ready(Ok(())) => {} + } + Pin::new(self.state.as_mut()).poll_write_vectored(cx, bufs) } fn poll_flush( - self: Pin<&mut Self>, - _cx: &mut std::task::Context<'_>, + mut self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, ) -> Poll> { - tracing::warn!( - path=%self.path.display(), - "Attempting to flush a readonly file", - ); - Poll::Ready(Err(std::io::ErrorKind::PermissionDenied.into())) + match self.poll_copy_start_and_progress(cx) { + Poll::Ready(Ok(())) => {} + p => return p, + } + Pin::new(self.state.as_mut()).poll_flush(cx) } fn poll_shutdown( - self: Pin<&mut Self>, - _cx: &mut std::task::Context<'_>, + mut self: Pin<&mut Self>, + cx: &mut std::task::Context<'_>, ) -> Poll> { - tracing::warn!( - path=%self.path.display(), - "Attempting to shutdown a readonly file", - ); - Poll::Ready(Err(std::io::ErrorKind::PermissionDenied.into())) + match self.poll_copy_start_and_progress(cx) { + Poll::Ready(Ok(())) => {} + p => return p, + } + Pin::new(self.state.as_mut()).poll_shutdown(cx) } } - impl AsyncRead for ReadOnlyFile { + impl

AsyncRead for CopyOnWriteFile

+ where + P: FileSystem + 'static, + { fn poll_read( mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>, buf: &mut tokio::io::ReadBuf<'_>, ) -> Poll> { - Pin::new(&mut *self.inner).poll_read(cx, buf) + match self.poll_copy_progress(cx) { + Poll::Ready(Ok(())) => {} + p => return p, + } + Pin::new(self.state.as_mut()).poll_read(cx, buf) } } - impl AsyncSeek for ReadOnlyFile { + impl

AsyncSeek for CopyOnWriteFile

+ where + P: FileSystem + 'static, + { fn start_seek( mut self: Pin<&mut Self>, position: std::io::SeekFrom, ) -> std::io::Result<()> { - Pin::new(&mut *self.inner).start_seek(position) + match &mut self.state { + CowState::ReadOnly(file) + | CowState::SeekingGet(file) + | CowState::Error { src: file, .. } + | CowState::Copied(file) + | CowState::SeekingRestore { dst: file, .. } => { + Pin::new(file.as_mut()).start_seek(position) + } + CowState::SeekingSet { + original_offset, + src, + .. + } + | CowState::Copying { + original_offset, + src, + .. + } => { + *original_offset = match position { + SeekFrom::Current(delta) => original_offset + .checked_add_signed(delta) + .unwrap_or(*original_offset), + SeekFrom::Start(pos) => pos, + SeekFrom::End(pos) => src + .size() + .checked_add_signed(pos) + .unwrap_or(*original_offset), + }; + Ok(()) + } + } } fn poll_complete( mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>, ) -> Poll> { - Pin::new(&mut *self.inner).poll_complete(cx) - } - } - - for fs in secondaries.filesystems() { - match fs.new_open_options().options(conf.clone()).open(path) { - Ok(f) => { - return Ok(Box::new(ReadOnlyFile { - path: path.to_path_buf(), - inner: f, - })); + match self.poll_copy_progress(cx) { + Poll::Ready(Ok(())) => {} + Poll::Ready(Err(err)) => return Poll::Ready(Err(err)), + Poll::Pending => return Poll::Pending, } - Err(e) if should_continue(e) => continue, - other => return other, + Pin::new(self.state.as_mut()).poll_complete(cx) } } - Err(FsError::EntryNotFound) -} - -fn opening_would_require_mutations( - secondaries: &S, - path: &Path, - conf: &OpenOptionsConfig, -) -> bool -where - S: for<'a> FileSystems<'a> + Send + Sync, -{ - if conf.append || conf.write || conf.create_new | conf.truncate { - return true; - } - - if conf.create { - // Would we create the file if it doesn't exist yet? - let already_exists = secondaries - .filesystems() - .into_iter() - .any(|fs| ops::is_file(fs, path)); - - if !already_exists { - return true; - } - } - - false + tracing::trace!( + path=%path.display(), + options=?conf, + "Opening the file in copy-on-write mode", + ); + Ok(Box::new(CopyOnWriteFile::

{ + path: path.to_path_buf(), + primary: primary.clone(), + state: CowState::ReadOnly(file), + readable: conf.read, + append: conf.append, + new_size: None, + })) } impl Debug for OverlayFileSystem @@ -477,7 +1047,7 @@ mod tests { use bytes::Bytes; use tempfile::TempDir; - use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}; use webc::v1::{ParseOptions, WebCOwned}; use super::*; @@ -485,23 +1055,6 @@ mod tests { const PYTHON: &[u8] = include_bytes!("../../c-api/examples/assets/python-0.1.0.wasmer"); - #[test] - fn object_safe() { - fn _box_with_memfs( - fs: OverlayFileSystem>, - ) -> Box { - Box::new(fs) - } - - fn _arc(fs: OverlayFileSystem) -> Arc - where - A: FileSystem + 'static, - S: for<'a> FileSystems<'a> + Send + Sync + Debug + 'static, - { - Arc::new(fs) - } - } - #[tokio::test] async fn remove_directory() { let primary = MemFS::default(); @@ -541,10 +1094,11 @@ mod tests { ); // Try to remove something on one of the overlay filesystems - assert_eq!( - overlay.remove_dir(third).unwrap_err(), - FsError::PermissionDenied, - ); + assert_eq!(overlay.remove_dir(third), Ok(())); + + // It should no longer exist + assert_eq!(overlay.metadata(third).unwrap_err(), FsError::EntryNotFound); + assert!(ops::exists(&overlay.secondaries[0], third)); } @@ -791,9 +1345,7 @@ mod tests { } #[tokio::test] - async fn open_secondary_fs_files_in_write_mode_and_error_on_first_write() { - // TODO(Michael-F-Bryan): remove this test if/when we fix - // open_readonly_file_hack() + async fn open_secondary_fs_files_in_write_mode() { let primary = MemFS::default(); let secondary = MemFS::default(); ops::create_dir_all(&secondary, "/secondary").unwrap(); @@ -813,19 +1365,261 @@ mod tests { let mut buf = String::new(); f.read_to_string(&mut buf).await.unwrap(); assert_eq!(buf, "Hello, World!"); - // but trying to write will error out - assert_eq!( - f.write(b"..").await.unwrap_err().kind(), - std::io::ErrorKind::PermissionDenied, - ); - // Same with flushing and shutdown + f.seek(SeekFrom::Start(0)).await.unwrap(); + // next we will write a new set of bytes + f.set_len(0).unwrap(); + assert_eq!(f.write(b"Hi").await.unwrap(), 2); + // Same with flushing + assert_eq!(f.flush().await.unwrap(), (),); + + // if we now read it then the data should be different + buf = String::new(); + f.seek(SeekFrom::Start(0)).await.unwrap(); + f.read_to_string(&mut buf).await.unwrap(); + assert_eq!(buf, "Hi"); + drop(f); + + // including if we open it again + let mut f = fs + .new_open_options() + .read(true) + .open("/secondary/file.txt") + .unwrap(); + buf = String::new(); + f.read_to_string(&mut buf).await.unwrap(); + assert_eq!(buf, "Hi"); + } + + #[tokio::test] + async fn open_secondary_fs_files_unlink() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::write(&secondary, "/secondary/file.txt", b"Hello, World!") + .await + .unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + fs.metadata(Path::new("/secondary/file.txt")).unwrap(); + + // Now delete the file and make sure its not found + fs.remove_file(Path::new("/secondary/file.txt")).unwrap(); assert_eq!( - f.flush().await.unwrap_err().kind(), - std::io::ErrorKind::PermissionDenied, - ); + fs.metadata(Path::new("/secondary/file.txt")).unwrap_err(), + FsError::EntryNotFound + ) + } + + #[tokio::test] + async fn open_secondary_fs_without_cow() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::write(&secondary, "/secondary/file.txt", b"Hello, World!") + .await + .unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + let mut f = fs + .new_open_options() + .create(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 13); + + let mut buf = String::new(); + f.read_to_string(&mut buf).await.unwrap(); + assert_eq!(buf, "Hello, World!"); + + // it should not be in the primary and nor should the secondary folder + assert!(!ops::is_dir(&fs.primary, "/secondary")); + assert!(!ops::is_file(&fs.primary, "/secondary/file.txt")); + assert!(ops::is_dir(&fs.secondaries[0], "/secondary")); + assert!(ops::is_file(&fs.secondaries[0], "/secondary/file.txt")); + } + + #[tokio::test] + async fn create_and_append_secondary_fs_with_cow() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::write(&secondary, "/secondary/file.txt", b"Hello, World!") + .await + .unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + let mut f = fs + .new_open_options() + .create(true) + .append(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 13); + + f.write_all(b"asdf").await.unwrap(); + assert_eq!(f.size() as usize, 17); + + f.seek(SeekFrom::Start(0)).await.unwrap(); + + let mut buf = String::new(); + f.read_to_string(&mut buf).await.unwrap(); + assert_eq!(buf, "Hello, World!asdf"); + + // Now lets check the file systems under + let f = fs + .primary + .new_open_options() + .create(true) + .append(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 17); + let f = fs.secondaries[0] + .new_open_options() + .create(true) + .append(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 13); + + // it should now exist in both the primary and secondary + assert!(ops::is_dir(&fs.primary, "/secondary")); + assert!(ops::is_file(&fs.primary, "/secondary/file.txt")); + assert!(ops::is_dir(&fs.secondaries[0], "/secondary")); + assert!(ops::is_file(&fs.secondaries[0], "/secondary/file.txt")); + } + + #[tokio::test] + async fn unlink_file_from_secondary_fs() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::write(&secondary, "/secondary/file.txt", b"Hello, World!") + .await + .unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + fs.remove_file(Path::new("/secondary/file.txt")).unwrap(); + assert_eq!(ops::exists(&fs, Path::new("/secondary/file.txt")), false); + + assert!(ops::is_file(&fs.primary, "/secondary/.wh.file.txt")); + assert!(ops::is_file(&fs.secondaries[0], "/secondary/file.txt")); + + // Now create the file again after the unlink + let mut f = fs + .new_open_options() + .create(true) + .write(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 0); + f.write_all(b"asdf").await.unwrap(); + assert_eq!(f.size() as usize, 4); + + // The whiteout should be gone and new file exist + assert!(!ops::is_file(&fs.primary, "/secondary/.wh.file.txt")); + assert!(ops::is_file(&fs.primary, "/secondary/file.txt")); + assert!(ops::is_file(&fs.secondaries[0], "/secondary/file.txt")); + } + + #[tokio::test] + async fn rmdir_from_secondary_fs() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + assert!(ops::is_dir(&fs, "/secondary")); + fs.remove_dir(Path::new("/secondary")).unwrap(); + + assert!(!ops::is_dir(&fs, "/secondary")); + assert!(ops::is_file(&fs.primary, "/.wh.secondary")); + assert!(ops::is_dir(&fs.secondaries[0], "/secondary")); + + fs.create_dir(Path::new("/secondary")).unwrap(); + assert!(ops::is_dir(&fs, "/secondary")); + assert!(ops::is_dir(&fs.primary, "/secondary")); + assert!(!ops::is_file(&fs.primary, "/.wh.secondary")); + assert!(ops::is_dir(&fs.secondaries[0], "/secondary")); + } + + #[tokio::test] + async fn rmdir_sub_from_secondary_fs() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/first/secondary").unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + assert!(ops::is_dir(&fs, "/first/secondary")); + fs.remove_dir(Path::new("/first/secondary")).unwrap(); + + assert!(!ops::is_dir(&fs, "/first/secondary")); + assert!(ops::is_file(&fs.primary, "/first/.wh.secondary")); + assert!(ops::is_dir(&fs.secondaries[0], "/first/secondary")); + + fs.create_dir(Path::new("/first/secondary")).unwrap(); + assert!(ops::is_dir(&fs, "/first/secondary")); + assert!(ops::is_dir(&fs.primary, "/first/secondary")); + assert!(!ops::is_file(&fs.primary, "/first/.wh.secondary")); + assert!(ops::is_dir(&fs.secondaries[0], "/first/secondary")); + } + + #[tokio::test] + async fn create_new_secondary_fs_without_cow() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::write(&secondary, "/secondary/file.txt", b"Hello, World!") + .await + .unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + let mut f = fs + .new_open_options() + .create_new(true) + .read(true) + .open(Path::new("/secondary/file.txt")) + .unwrap(); + assert_eq!(f.size() as usize, 0); + + let mut buf = String::new(); + f.read_to_string(&mut buf).await.unwrap(); + assert_eq!(buf, ""); + + // it should now exist in both the primary and secondary + assert!(ops::is_dir(&fs.primary, "/secondary")); + assert!(ops::is_file(&fs.primary, "/secondary/file.txt")); + assert!(ops::is_dir(&fs.secondaries[0], "/secondary")); + assert!(ops::is_file(&fs.secondaries[0], "/secondary/file.txt")); + } + + #[tokio::test] + async fn open_secondary_fs_files_remove_dir() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + + let fs = OverlayFileSystem::new(primary, [secondary]); + + fs.metadata(Path::new("/secondary")).unwrap(); + + // Now delete the file and make sure its not found + fs.remove_dir(Path::new("/secondary")).unwrap(); assert_eq!( - f.shutdown().await.unwrap_err().kind(), - std::io::ErrorKind::PermissionDenied, - ); + fs.metadata(Path::new("/secondary")).unwrap_err(), + FsError::EntryNotFound + ) } } diff --git a/lib/virtual-fs/src/passthru_fs.rs b/lib/virtual-fs/src/passthru_fs.rs index 23a8d7bc46a..08a940ed0c5 100644 --- a/lib/virtual-fs/src/passthru_fs.rs +++ b/lib/virtual-fs/src/passthru_fs.rs @@ -3,8 +3,6 @@ //! shared - some of the interfaces pass around a `Box` use std::path::Path; -#[allow(unused_imports, dead_code)] -use tracing::{debug, error, info, trace, warn}; use crate::*; @@ -32,8 +30,8 @@ impl FileSystem for PassthruFileSystem { self.fs.remove_dir(path) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - self.fs.rename(from, to) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { self.fs.rename(from, to).await }) } fn metadata(&self, path: &Path) -> Result { diff --git a/lib/virtual-fs/src/pipe.rs b/lib/virtual-fs/src/pipe.rs index 15146c5d19d..32fcfa9f220 100644 --- a/lib/virtual-fs/src/pipe.rs +++ b/lib/virtual-fs/src/pipe.rs @@ -1,4 +1,5 @@ use bytes::{Buf, Bytes}; +use futures::future::BoxFuture; #[cfg(feature = "futures")] use futures::Future; use std::io::IoSlice; @@ -384,13 +385,13 @@ impl VirtualFile for Pipe { /// Change the size of the file, if the `new_size` is greater than the current size /// the extra bytes will be allocated and zeroed - fn set_len(&mut self, _new_size: u64) -> Result<(), FsError> { + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } /// Request deletion of the file - fn unlink(&mut self) -> Result<(), FsError> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { + Box::pin(async { Ok(()) }) } /// Indicates if the file is opened or closed. This function must not block diff --git a/lib/virtual-fs/src/random_file.rs b/lib/virtual-fs/src/random_file.rs index 5722859ad00..98db4d128cc 100644 --- a/lib/virtual-fs/src/random_file.rs +++ b/lib/virtual-fs/src/random_file.rs @@ -5,6 +5,7 @@ use std::io::{self, *}; use std::pin::Pin; use std::task::{Context, Poll}; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use crate::VirtualFile; @@ -76,8 +77,8 @@ impl VirtualFile for RandomFile { fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { Poll::Ready(Ok(0)) diff --git a/lib/virtual-fs/src/special_file.rs b/lib/virtual-fs/src/special_file.rs index a726e464f91..079974e1873 100644 --- a/lib/virtual-fs/src/special_file.rs +++ b/lib/virtual-fs/src/special_file.rs @@ -6,6 +6,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use crate::VirtualFile; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; pub type Fd = u32; @@ -93,8 +94,8 @@ impl VirtualFile for DeviceFile { fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Ok(()) }) } fn get_special_fd(&self) -> Option { Some(self.fd) diff --git a/lib/virtual-fs/src/static_fs.rs b/lib/virtual-fs/src/static_fs.rs index ddb9c4a27d3..e18da882ce0 100644 --- a/lib/virtual-fs/src/static_fs.rs +++ b/lib/virtual-fs/src/static_fs.rs @@ -1,4 +1,5 @@ use anyhow::anyhow; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use std::convert::TryInto; @@ -113,11 +114,11 @@ impl VirtualFile for WebCFile { fn size(&self) -> u64 { self.entry.get_len() } - fn set_len(&mut self, _new_size: u64) -> Result<(), FsError> { + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> Result<(), FsError> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { let remaining = self.entry.get_len() - self.cursor; @@ -276,20 +277,22 @@ impl FileSystem for StaticFileSystem { result } } - fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - let from = normalizes_path(from); - let to = normalizes_path(to); - let result = self.memory.rename(Path::new(&from), Path::new(&to)); - if self - .volumes - .values() - .find_map(|v| v.get_file_entry(&from).ok()) - .is_some() - { - Ok(()) - } else { - result - } + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<(), FsError>> { + Box::pin(async { + let from = normalizes_path(from); + let to = normalizes_path(to); + let result = self.memory.rename(Path::new(&from), Path::new(&to)).await; + if self + .volumes + .values() + .find_map(|v| v.get_file_entry(&from).ok()) + .is_some() + { + Ok(()) + } else { + result + } + }) } fn metadata(&self, path: &Path) -> Result { let path = normalizes_path(path); diff --git a/lib/virtual-fs/src/tmp_fs.rs b/lib/virtual-fs/src/tmp_fs.rs index e72c8d33b9d..46e47f0fe53 100644 --- a/lib/virtual-fs/src/tmp_fs.rs +++ b/lib/virtual-fs/src/tmp_fs.rs @@ -2,23 +2,15 @@ //! enhanced to support mounting file systems, shared static files, //! readonly files, etc... -#![allow(dead_code)] -#![allow(unused)] -use std::collections::HashMap; -use std::io::prelude::*; -use std::io::SeekFrom; -use std::io::{self}; -use std::path::{Path, PathBuf}; -use std::result::Result as StdResult; -use std::sync::atomic::AtomicU32; -use std::sync::Arc; -use std::sync::Mutex; -#[allow(unused_imports, dead_code)] -use tracing::{debug, error, info, trace, warn}; - -use crate::mem_fs; -use crate::Result as FsResult; -use crate::*; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; + +use crate::{ + limiter::DynFsMemoryLimiter, mem_fs, BoxFuture, FileSystem, Metadata, OpenOptions, ReadDir, + Result, +}; #[derive(Debug, Default, Clone)] pub struct TmpFileSystem { @@ -30,7 +22,7 @@ impl TmpFileSystem { Self::default() } - pub fn set_memory_limiter(&self, limiter: crate::limiter::DynFsMemoryLimiter) { + pub fn set_memory_limiter(&self, limiter: DynFsMemoryLimiter) { self.fs.set_memory_limiter(limiter); } @@ -81,8 +73,8 @@ impl FileSystem for TmpFileSystem { self.fs.remove_dir(path) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - self.fs.rename(from, to) + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { self.fs.rename(from, to).await }) } fn metadata(&self, path: &Path) -> Result { diff --git a/lib/virtual-fs/src/trace_fs.rs b/lib/virtual-fs/src/trace_fs.rs index d6346f9a463..627dca8a5b3 100644 --- a/lib/virtual-fs/src/trace_fs.rs +++ b/lib/virtual-fs/src/trace_fs.rs @@ -4,6 +4,7 @@ use std::{ task::{Context, Poll}, }; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite, ReadBuf}; use crate::{FileOpener, FileSystem, OpenOptionsConfig, VirtualFile}; @@ -54,8 +55,12 @@ where } #[tracing::instrument(level = "trace", skip(self), err)] - fn rename(&self, from: &std::path::Path, to: &std::path::Path) -> crate::Result<()> { - self.0.rename(from, to) + fn rename<'a>( + &'a self, + from: &'a std::path::Path, + to: &'a std::path::Path, + ) -> BoxFuture<'a, crate::Result<()>> { + Box::pin(async { self.0.rename(from, to).await }) } #[tracing::instrument(level = "trace", skip(self), err)] @@ -85,10 +90,9 @@ where conf: &OpenOptionsConfig, ) -> crate::Result> { let file = self.0.new_open_options().options(conf.clone()).open(path)?; - Ok(Box::new(TraceFile { file, - path: path.to_path_buf(), + path: path.to_owned(), })) } } @@ -125,9 +129,9 @@ impl VirtualFile for TraceFile { self.file.set_len(new_size) } - #[tracing::instrument(level = "trace", skip(self), fields(path=%self.path.display()), err)] - fn unlink(&mut self) -> crate::Result<()> { - self.file.unlink() + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + let fut = self.file.unlink(); + Box::pin(async move { fut.await }) } #[tracing::instrument(level = "trace", skip_all, fields(path=%self.path.display()))] diff --git a/lib/virtual-fs/src/union_fs.rs b/lib/virtual-fs/src/union_fs.rs index d85db2da04a..57519f32584 100644 --- a/lib/virtual-fs/src/union_fs.rs +++ b/lib/virtual-fs/src/union_fs.rs @@ -2,18 +2,14 @@ //! its not as simple as TmpFs. not currently used but was used by //! the previoulsy implementation of Deploy - now using TmpFs -#![allow(dead_code)] -#![allow(unused)] use crate::*; -use std::borrow::Cow; -use std::ops::Add; -use std::path::{Path, PathBuf}; -use std::sync::atomic::AtomicU32; -use std::sync::Arc; -use std::sync::Mutex; -use std::sync::Weak; -#[allow(unused_imports, dead_code)] -use tracing::{debug, error, info, trace, warn}; + +use std::{ + path::Path, + sync::{Arc, Mutex, Weak}, +}; + +use tracing::{debug, trace}; pub type TempHolding = Arc>>>>; @@ -262,42 +258,48 @@ impl FileSystem for UnionFileSystem { } Err(ret_error) } - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - println!("rename: from={} to={}", from.display(), to.display()); - if from.parent().is_none() { - return Err(FsError::BaseNotDirectory); - } - if to.parent().is_none() { - return Err(FsError::BaseNotDirectory); - } - let mut ret_error = FsError::EntryNotFound; - let from = from.to_string_lossy(); - let to = to.to_string_lossy(); - #[cfg(target_os = "windows")] - let to = to.replace('\\', "/"); - for (path, mount) in filter_mounts(&self.mounts, from.as_ref()) { - let mut to = if to.starts_with(mount.path.as_str()) { - (to[mount.path.len()..]).to_string() - } else { - ret_error = FsError::UnknownError; - continue; - }; - if !to.starts_with('/') { - to = format!("/{}", to); + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<()>> { + Box::pin(async { + println!("rename: from={} to={}", from.display(), to.display()); + if from.parent().is_none() { + return Err(FsError::BaseNotDirectory); } - match mount.fs.rename(Path::new(&path), Path::new(to.as_str())) { - Ok(ret) => { - trace!("rename ok"); - return Ok(ret); + if to.parent().is_none() { + return Err(FsError::BaseNotDirectory); + } + let mut ret_error = FsError::EntryNotFound; + let from = from.to_string_lossy(); + let to = to.to_string_lossy(); + #[cfg(target_os = "windows")] + let to = to.replace('\\', "/"); + for (path, mount) in filter_mounts(&self.mounts, from.as_ref()) { + let mut to = if to.starts_with(mount.path.as_str()) { + (to[mount.path.len()..]).to_string() + } else { + ret_error = FsError::UnknownError; + continue; + }; + if !to.starts_with('/') { + to = format!("/{}", to); } - Err(err) => { - trace!("rename error (from={}, to={}) - {}", from, to, err); - ret_error = err; + match mount + .fs + .rename(Path::new(&path), Path::new(to.as_str())) + .await + { + Ok(ret) => { + trace!("rename ok"); + return Ok(ret); + } + Err(err) => { + trace!("rename error (from={}, to={}) - {}", from, to, err); + ret_error = err; + } } } - } - trace!("rename failed - {}", ret_error); - Err(ret_error) + trace!("rename failed - {}", ret_error); + Err(ret_error) + }) } fn metadata(&self, path: &Path) -> Result { debug!("metadata: path={}", path.display()); @@ -372,7 +374,7 @@ impl FileSystem for UnionFileSystem { fn filter_mounts( mounts: &[MountPoint], - mut target: &str, + target: &str, ) -> impl Iterator { // On Windows, Path might use \ instead of /, wich mill messup the matching of mount points #[cfg(target_os = "windows")] @@ -412,7 +414,7 @@ fn filter_mounts( } } } - ret.retain(|(a, b)| b.path.len() >= biggest_path); + ret.retain(|(_, b)| b.path.len() >= biggest_path); ret.into_iter() } @@ -460,13 +462,11 @@ impl FileOpener for UnionFileSystem { #[cfg(test)] mod tests { - use std::{path::Path, sync::Arc}; + use std::path::Path; use tokio::io::AsyncWriteExt; - use crate::{ - host_fs::FileSystem, mem_fs, ops, FileSystem as FileSystemTrait, FsError, UnionFileSystem, - }; + use crate::{mem_fs, ops, FileSystem as FileSystemTrait, FsError, UnionFileSystem}; fn gen_filesystem() -> UnionFileSystem { let mut union = UnionFileSystem::new(); @@ -648,12 +648,12 @@ mod tests { let _ = fs_extra::remove_items(&["./test_rename"]); assert_eq!( - fs.rename(Path::new("/"), Path::new("/bar")), + fs.rename(Path::new("/"), Path::new("/bar")).await, Err(FsError::BaseNotDirectory), "renaming a directory that has no parent", ); assert_eq!( - fs.rename(Path::new("/foo"), Path::new("/")), + fs.rename(Path::new("/foo"), Path::new("/")).await, Err(FsError::BaseNotDirectory), "renaming to a directory that has no parent", ); @@ -666,7 +666,8 @@ mod tests { fs.rename( Path::new("/test_rename/foo"), Path::new("/test_rename/bar/baz") - ), + ) + .await, Err(FsError::EntryNotFound), "renaming to a directory that has parent that doesn't exist", ); @@ -674,7 +675,8 @@ mod tests { assert_eq!(fs.create_dir(Path::new("/test_rename/bar")), Ok(())); assert_eq!( - fs.rename(Path::new("/test_rename/foo"), Path::new("/test_rename/bar")), + fs.rename(Path::new("/test_rename/foo"), Path::new("/test_rename/bar")) + .await, Ok(()), "renaming to a directory that has parent that exists", ); @@ -742,7 +744,8 @@ mod tests { fs.rename( Path::new("/test_rename/bar/hello2.txt"), Path::new("/test_rename/foo/world2.txt") - ), + ) + .await, Ok(()), "renaming (and moving) a file", ); @@ -751,7 +754,8 @@ mod tests { fs.rename( Path::new("/test_rename/foo"), Path::new("/test_rename/bar/baz") - ), + ) + .await, Ok(()), "renaming a directory", ); @@ -760,7 +764,8 @@ mod tests { fs.rename( Path::new("/test_rename/bar/hello1.txt"), Path::new("/test_rename/bar/world1.txt") - ), + ) + .await, Ok(()), "renaming a file (in the same directory)", ); @@ -837,7 +842,8 @@ mod tests { fs.rename( Path::new("/test_metadata/foo"), Path::new("/test_metadata/bar") - ), + ) + .await, Ok(()) ); diff --git a/lib/virtual-fs/src/webc_fs.rs b/lib/virtual-fs/src/webc_fs.rs index 96cb806972a..0a67c739bfa 100644 --- a/lib/virtual-fs/src/webc_fs.rs +++ b/lib/virtual-fs/src/webc_fs.rs @@ -1,19 +1,23 @@ -use crate::mem_fs::FileSystem as MemFileSystem; -use crate::{ - FileOpener, FileSystem, FsError, Metadata, OpenOptions, OpenOptionsConfig, ReadDir, VirtualFile, +use std::{ + convert::{TryFrom, TryInto}, + io::{self, Error as IoError, ErrorKind as IoErrorKind, SeekFrom}, + ops::Deref, + path::{Path, PathBuf}, + pin::Pin, + sync::Arc, + task::{Context, Poll}, }; + use anyhow::anyhow; -use std::convert::{TryFrom, TryInto}; -use std::io::{self, Error as IoError, ErrorKind as IoErrorKind, SeekFrom}; -use std::ops::Deref; -use std::path::Path; -use std::path::PathBuf; -use std::pin::Pin; -use std::sync::Arc; -use std::task::{Context, Poll}; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use webc::v1::{FsEntry, FsEntryType, OwnedFsEntryFile, WebC}; +use crate::{ + mem_fs::FileSystem as MemFileSystem, FileOpener, FileSystem, FsError, Metadata, OpenOptions, + OpenOptionsConfig, ReadDir, VirtualFile, +}; + /// Custom file system wrapper to map requested file paths #[derive(Debug)] pub struct WebcFileSystem @@ -157,11 +161,11 @@ where fn size(&self) -> u64 { self.entry.get_len() } - fn set_len(&mut self, _new_size: u64) -> Result<(), FsError> { + fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> Result<(), FsError> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { let remaining = self.entry.get_len() - self.cursor; @@ -329,15 +333,17 @@ where result } } - fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - let from = normalizes_path(from); - let to = normalizes_path(to); - let result = self.memory.rename(Path::new(&from), Path::new(&to)); - if self.volumes.iter().any(|v| v.get_file_entry(&from).is_ok()) { - Ok(()) - } else { - result - } + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<(), FsError>> { + Box::pin(async { + let from = normalizes_path(from); + let to = normalizes_path(to); + let result = self.memory.rename(Path::new(&from), Path::new(&to)).await; + if self.volumes.iter().any(|v| v.get_file_entry(&from).is_ok()) { + Ok(()) + } else { + result + } + }) } fn metadata(&self, path: &Path) -> Result { let path = normalizes_path(path); diff --git a/lib/virtual-fs/src/webc_volume_fs.rs b/lib/virtual-fs/src/webc_volume_fs.rs index 3f829f621e7..56d0a43907f 100644 --- a/lib/virtual-fs/src/webc_volume_fs.rs +++ b/lib/virtual-fs/src/webc_volume_fs.rs @@ -7,6 +7,7 @@ use std::{ task::Poll, }; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use webc::{ compat::{Container, SharedBytes, Volume}, @@ -107,19 +108,21 @@ impl FileSystem for WebcVolumeFileSystem { Err(FsError::PermissionDenied) } - fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - // The original file should exist - let _ = self.metadata(from)?; + fn rename<'a>(&'a self, from: &'a Path, to: &'a Path) -> BoxFuture<'a, Result<(), FsError>> { + Box::pin(async { + // The original file should exist + let _ = self.metadata(from)?; - // we also want to make sure the destination's folder exists, too - let dest_parent = to.parent().unwrap_or_else(|| Path::new("/")); - let parent_meta = self.metadata(dest_parent)?; - if !parent_meta.is_dir() { - return Err(FsError::BaseNotDirectory); - } + // we also want to make sure the destination's folder exists, too + let dest_parent = to.parent().unwrap_or_else(|| Path::new("/")); + let parent_meta = self.metadata(dest_parent)?; + if !parent_meta.is_dir() { + return Err(FsError::BaseNotDirectory); + } - // but we are a readonly filesystem, so you can't modify anything - Err(FsError::PermissionDenied) + // but we are a readonly filesystem, so you can't modify anything + Err(FsError::PermissionDenied) + }) } fn metadata(&self, path: &Path) -> Result { @@ -204,8 +207,8 @@ impl VirtualFile for File { Err(FsError::PermissionDenied) } - fn unlink(&mut self) -> crate::Result<()> { - Err(FsError::PermissionDenied) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Err(FsError::PermissionDenied) }) } fn poll_read_ready( @@ -616,8 +619,8 @@ mod tests { ); } - #[test] - fn rename_is_not_allowed() { + #[tokio::test] + async fn rename_is_not_allowed() { let container = Container::from_bytes(PYTHON_WEBC).unwrap(); let volumes = container.volumes(); let volume = volumes["atom"].clone(); @@ -625,16 +628,20 @@ mod tests { let fs = WebcVolumeFileSystem::new(volume); assert_eq!( - fs.rename("/lib".as_ref(), "/other".as_ref()).unwrap_err(), + fs.rename("/lib".as_ref(), "/other".as_ref()) + .await + .unwrap_err(), FsError::PermissionDenied, ); assert_eq!( fs.rename("/this/does/not/exist".as_ref(), "/another".as_ref()) + .await .unwrap_err(), FsError::EntryNotFound, ); assert_eq!( fs.rename("/lib/python.wasm".as_ref(), "/lib/another.wasm".as_ref()) + .await .unwrap_err(), FsError::PermissionDenied, ); diff --git a/lib/virtual-fs/src/zero_file.rs b/lib/virtual-fs/src/zero_file.rs index f14777897d6..e28ff98ffd5 100644 --- a/lib/virtual-fs/src/zero_file.rs +++ b/lib/virtual-fs/src/zero_file.rs @@ -1,11 +1,14 @@ //! Used for /dev/zero - infinitely returns zero //! which is useful for commands like `dd if=/dev/zero of=bigfile.img size=1G` -use std::io::{self, *}; -use std::iter; -use std::pin::Pin; -use std::task::{Context, Poll}; +use std::{ + io::{self, IoSlice, SeekFrom}, + iter, + pin::Pin, + task::{Context, Poll}, +}; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use crate::VirtualFile; @@ -76,8 +79,8 @@ impl VirtualFile for ZeroFile { fn set_len(&mut self, _new_size: u64) -> crate::Result<()> { Ok(()) } - fn unlink(&mut self) -> crate::Result<()> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, crate::Result<()>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { Poll::Ready(Ok(0)) diff --git a/lib/wasix/src/bin_factory/exec.rs b/lib/wasix/src/bin_factory/exec.rs index 894d255a372..ac8a64bb6d5 100644 --- a/lib/wasix/src/bin_factory/exec.rs +++ b/lib/wasix/src/bin_factory/exec.rs @@ -41,7 +41,7 @@ pub async fn spawn_exec( SpawnError::CompileError }); if module.is_err() { - env.blocking_cleanup(Some(Errno::Noexec.into())); + env.cleanup(Some(Errno::Noexec.into())).await; } let module = module?; @@ -57,13 +57,20 @@ pub async fn spawn_exec( } (None, None) => { error!("package has no entry [{}]", name,); - env.blocking_cleanup(Some(Errno::Noexec.into())); + env.cleanup(Some(Errno::Noexec.into())).await; return Err(SpawnError::CompileError); } }; // If the file system has not already been union'ed then do so - env.state.fs.conditional_union(&binary); + env.state + .fs + .conditional_union(&binary) + .await + .map_err(|err| { + tracing::warn!("failed to union file system - {err}"); + SpawnError::FileSystemError + })?; tracing::debug!("{:?}", env.state.fs); // Now run the module diff --git a/lib/wasix/src/fs/inode_guard.rs b/lib/wasix/src/fs/inode_guard.rs index 1df3554a36b..da3995a9a5c 100644 --- a/lib/wasix/src/fs/inode_guard.rs +++ b/lib/wasix/src/fs/inode_guard.rs @@ -8,6 +8,7 @@ use std::{ task::{Context, Poll}, }; +use futures::future::BoxFuture; use tokio::io::{AsyncRead, AsyncSeek, AsyncWrite}; use virtual_fs::{FsError, Pipe as VirtualPipe, VirtualFile}; use virtual_net::NetworkError; @@ -587,13 +588,19 @@ impl VirtualFile for WasiStateFileGuard { } } - fn unlink(&mut self) -> Result<(), FsError> { + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { let mut guard = self.lock_write(); - if let Some(file) = guard.as_mut() { - file.unlink() + let fut = if let Some(file) = guard.as_mut() { + Ok(file.unlink()) } else { Err(FsError::IOError) - } + }; + Box::pin(async move { + match fut { + Ok(fut) => fut.await, + Err(err) => Err(err), + } + }) } fn is_open(&self) -> bool { diff --git a/lib/wasix/src/fs/mod.rs b/lib/wasix/src/fs/mod.rs index a581fcf9a37..deec998e59e 100644 --- a/lib/wasix/src/fs/mod.rs +++ b/lib/wasix/src/fs/mod.rs @@ -14,12 +14,12 @@ use std::{ }; use crate::state::{Stderr, Stdin, Stdout}; -use futures::TryStreamExt; +use futures::{future::BoxFuture, TryStreamExt}; #[cfg(feature = "enable-serde")] use serde_derive::{Deserialize, Serialize}; use tokio::io::AsyncWriteExt; use tracing::{debug, trace}; -use virtual_fs::{FileSystem, FsError, OpenOptions, VirtualFile}; +use virtual_fs::{copy_reference, FileSystem, FsError, OpenOptions, VirtualFile}; use wasmer_wasix_types::{ types::{__WASI_STDERR_FILENO, __WASI_STDIN_FILENO, __WASI_STDOUT_FILENO}, wasi::{ @@ -307,11 +307,16 @@ impl FileSystem for WasiFsRoot { WasiFsRoot::Backing(fs) => fs.remove_dir(path), } } - fn rename(&self, from: &Path, to: &Path) -> virtual_fs::Result<()> { - match self { - WasiFsRoot::Sandbox(fs) => fs.rename(from, to), - WasiFsRoot::Backing(fs) => fs.rename(from, to), - } + fn rename<'a>(&'a self, from: &Path, to: &Path) -> BoxFuture<'a, virtual_fs::Result<()>> { + let from = from.to_owned(); + let to = to.to_owned(); + let this = self.clone(); + Box::pin(async move { + match this { + WasiFsRoot::Sandbox(fs) => fs.rename(&from, &to).await, + WasiFsRoot::Backing(fs) => fs.rename(&from, &to).await, + } + }) } fn metadata(&self, path: &Path) -> virtual_fs::Result { match self { @@ -380,23 +385,6 @@ async fn merge_filesystems( files.try_collect().await } -#[tracing::instrument(level = "trace", skip_all, fields(path=%path.display()))] -async fn copy_reference( - source: &dyn FileSystem, - destination: &dyn FileSystem, - path: &Path, -) -> Result<(), std::io::Error> { - let src = source.new_open_options().read(true).open(path)?; - let mut dst = destination - .new_open_options() - .create(true) - .write(true) - .truncate(true) - .open(path)?; - - dst.copy_reference(src).await -} - fn create_dir_all(fs: &dyn FileSystem, path: &Path) -> Result<(), virtual_fs::FsError> { if fs.metadata(path).is_ok() { return Ok(()); @@ -489,21 +477,28 @@ impl WasiFs { /// Will conditionally union the binary file system with this one /// if it has not already been unioned - pub fn conditional_union(&self, binary: &BinaryPackage) -> bool { - let sandbox_fs = match &self.root_fs { - WasiFsRoot::Sandbox(fs) => fs, - WasiFsRoot::Backing(_) => { - tracing::error!("can not perform a union on a backing file system"); - return false; + pub async fn conditional_union( + &self, + binary: &BinaryPackage, + ) -> Result<(), virtual_fs::FsError> { + let package_name = binary.package_name.clone(); + + let needs_to_be_unioned = self.has_unioned.lock().unwrap().insert(package_name); + + if !needs_to_be_unioned { + return Ok(()); + } + + match self.root_fs { + WasiFsRoot::Sandbox(ref sandbox_fs) => { + sandbox_fs.union(&binary.webc_fs); + } + WasiFsRoot::Backing(ref fs) => { + merge_filesystems(&binary.webc_fs, fs.deref()).await?; } - }; - let package_name = binary.package_name.to_string(); - let mut guard = self.has_unioned.lock().unwrap(); - if !guard.contains(&package_name) { - guard.insert(package_name); - sandbox_fs.union(&binary.webc_fs); } - true + + Ok(()) } /// Created for the builder API. like `new` but with more information @@ -1897,7 +1892,7 @@ impl FileSystem for FallbackFileSystem { fn remove_dir(&self, _path: &Path) -> Result<(), FsError> { Self::fail(); } - fn rename(&self, _from: &Path, _to: &Path) -> Result<(), FsError> { + fn rename<'a>(&'a self, _from: &Path, _to: &Path) -> BoxFuture<'a, Result<(), FsError>> { Self::fail(); } fn metadata(&self, _path: &Path) -> Result { diff --git a/lib/wasix/src/lib.rs b/lib/wasix/src/lib.rs index 09657270177..d18ef45cdcc 100644 --- a/lib/wasix/src/lib.rs +++ b/lib/wasix/src/lib.rs @@ -155,9 +155,12 @@ pub enum SpawnError { /// Access denied #[error("access denied")] AccessDenied, - /// Internal error has occured + /// Internal error has occurred #[error("internal error")] InternalError, + /// An error occurred while preparing the file system + #[error("file system error")] + FileSystemError, /// Memory allocation failed #[error("memory allocation failed")] MemoryAllocationFailed, diff --git a/lib/wasix/src/runners/wasi_common.rs b/lib/wasix/src/runners/wasi_common.rs index 5498de74f35..99758bb5525 100644 --- a/lib/wasix/src/runners/wasi_common.rs +++ b/lib/wasix/src/runners/wasi_common.rs @@ -5,6 +5,7 @@ use std::{ }; use anyhow::{Context, Error}; +use futures::future::BoxFuture; use virtual_fs::{FileSystem, FsError, OverlayFileSystem, RootFileSystemBuilder}; use webc::metadata::annotations::Wasi as WasiAnnotation; @@ -162,8 +163,9 @@ fn prepare_filesystem( // supported or not, we'll add an adapter that automatically retries // operations using an absolute path if it failed using a relative path. let container_fs = RelativeOrAbsolutePathHack(container_fs); + let fs = OverlayFileSystem::new(root_fs, [container_fs]); - Ok(Box::new(OverlayFileSystem::new(root_fs, [container_fs]))) + Ok(Box::new(fs)) } /// HACK: We need this so users can mount host directories at relative paths. @@ -245,8 +247,10 @@ impl virtual_fs::FileSystem for RelativeOrAbsolutePathHack { self.execute(path, |fs, p| fs.remove_dir(p)) } - fn rename(&self, from: &Path, to: &Path) -> virtual_fs::Result<()> { - self.execute(from, |fs, p| fs.rename(p, to)) + fn rename<'a>(&'a self, from: &Path, to: &Path) -> BoxFuture<'a, virtual_fs::Result<()>> { + let from = from.to_owned(); + let to = to.to_owned(); + Box::pin(async move { self.0.rename(&from, &to).await }) } fn metadata(&self, path: &Path) -> virtual_fs::Result { diff --git a/lib/wasix/src/runtime/package_loader/load_package_tree.rs b/lib/wasix/src/runtime/package_loader/load_package_tree.rs index 7fa9c105b54..969461e0a4d 100644 --- a/lib/wasix/src/runtime/package_loader/load_package_tree.rs +++ b/lib/wasix/src/runtime/package_loader/load_package_tree.rs @@ -6,7 +6,7 @@ use std::{ }; use anyhow::{Context, Error}; -use futures::{StreamExt, TryStreamExt}; +use futures::{future::BoxFuture, StreamExt, TryStreamExt}; use once_cell::sync::OnceCell; use virtual_fs::{FileSystem, OverlayFileSystem, WebcVolumeFileSystem}; use webc::compat::{Container, Volume}; @@ -390,10 +390,14 @@ where self.inner.remove_dir(&path) } - fn rename(&self, from: &Path, to: &Path) -> virtual_fs::Result<()> { - let from = self.path(from)?; - let to = self.path(to)?; - self.inner.rename(&from, &to) + fn rename<'a>(&'a self, from: &Path, to: &Path) -> BoxFuture<'a, virtual_fs::Result<()>> { + let from = from.to_owned(); + let to = to.to_owned(); + Box::pin(async move { + let from = self.path(&from)?; + let to = self.path(&to)?; + self.inner.rename(&from, &to).await + }) } fn metadata(&self, path: &Path) -> virtual_fs::Result { diff --git a/lib/wasix/src/state/mod.rs b/lib/wasix/src/state/mod.rs index 0a354f9c30d..4ec76f769c4 100644 --- a/lib/wasix/src/state/mod.rs +++ b/lib/wasix/src/state/mod.rs @@ -179,7 +179,7 @@ impl WasiState { .map_err(fs_error_into_wasi_err) } - pub(crate) fn fs_rename, Q: AsRef>( + pub(crate) async fn fs_rename, Q: AsRef>( &self, from: P, to: Q, @@ -187,6 +187,7 @@ impl WasiState { self.fs .root_fs .rename(from.as_ref(), to.as_ref()) + .await .map_err(fs_error_into_wasi_err) } diff --git a/lib/wasix/src/syscalls/wasi/path_rename.rs b/lib/wasix/src/syscalls/wasi/path_rename.rs index 114328a2543..c33437bc4d0 100644 --- a/lib/wasix/src/syscalls/wasi/path_rename.rs +++ b/lib/wasix/src/syscalls/wasi/path_rename.rs @@ -25,31 +25,31 @@ pub fn path_rename( new_fd: WasiFd, new_path: WasmPtr, new_path_len: M::Offset, -) -> Errno { +) -> Result { let env = ctx.data(); let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) }; - let mut source_str = unsafe { get_input_str!(&memory, old_path, old_path_len) }; + let mut source_str = unsafe { get_input_str_ok!(&memory, old_path, old_path_len) }; Span::current().record("old_path", source_str.as_str()); source_str = ctx.data().state.fs.relative_path_to_absolute(source_str); let source_path = std::path::Path::new(&source_str); - let mut target_str = unsafe { get_input_str!(&memory, new_path, new_path_len) }; + let mut target_str = unsafe { get_input_str_ok!(&memory, new_path, new_path_len) }; Span::current().record("new_path", target_str.as_str()); target_str = ctx.data().state.fs.relative_path_to_absolute(target_str); let target_path = std::path::Path::new(&target_str); { - let source_fd = wasi_try!(state.fs.get_fd(old_fd)); + let source_fd = wasi_try_ok!(state.fs.get_fd(old_fd)); if !source_fd.rights.contains(Rights::PATH_RENAME_SOURCE) { - return Errno::Access; + return Ok(Errno::Access); } - let target_fd = wasi_try!(state.fs.get_fd(new_fd)); + let target_fd = wasi_try_ok!(state.fs.get_fd(new_fd)); if !target_fd.rights.contains(Rights::PATH_RENAME_TARGET) { - return Errno::Access; + return Ok(Errno::Access); } } // this is to be sure the source file is fetch from filesystem if needed - wasi_try!(state.fs.get_inode_at_path( + wasi_try_ok!(state.fs.get_inode_at_path( inodes, old_fd, source_path.to_str().as_ref().unwrap(), @@ -61,11 +61,11 @@ pub fn path_rename( .fs .get_inode_at_path(inodes, new_fd, target_path.to_str().as_ref().unwrap(), true); let (source_parent_inode, source_entry_name) = - wasi_try!(state + wasi_try_ok!(state .fs .get_parent_inode_at_path(inodes, old_fd, source_path, true)); let (target_parent_inode, target_entry_name) = - wasi_try!(state + wasi_try_ok!(state .fs .get_parent_inode_at_path(inodes, new_fd, target_path, true)); let mut need_create = true; @@ -80,13 +80,13 @@ pub fn path_rename( out_path.push(std::path::Path::new(&target_entry_name)); out_path } - Kind::Root { .. } => return Errno::Notcapable, + Kind::Root { .. } => return Ok(Errno::Notcapable), Kind::Socket { .. } | Kind::Pipe { .. } | Kind::EventNotifications { .. } => { - return Errno::Inval + return Ok(Errno::Inval) } Kind::Symlink { .. } | Kind::File { .. } | Kind::Buffer { .. } => { debug!("fatal internal logic error: parent of inode is not a directory"); - return Errno::Inval; + return Ok(Errno::Inval); } } }; @@ -95,15 +95,15 @@ pub fn path_rename( let mut guard = source_parent_inode.write(); match guard.deref_mut() { Kind::Dir { entries, .. } => { - wasi_try!(entries.remove(&source_entry_name).ok_or(Errno::Noent)) + wasi_try_ok!(entries.remove(&source_entry_name).ok_or(Errno::Noent)) } - Kind::Root { .. } => return Errno::Notcapable, + Kind::Root { .. } => return Ok(Errno::Notcapable), Kind::Socket { .. } | Kind::Pipe { .. } | Kind::EventNotifications { .. } => { - return Errno::Inval + return Ok(Errno::Inval); } Kind::Symlink { .. } | Kind::File { .. } | Kind::Buffer { .. } => { debug!("fatal internal logic error: parent of inode is not a directory"); - return Errno::Inval; + return Ok(Errno::Inval); } } }; @@ -121,11 +121,24 @@ pub fn path_rename( // implements the logic of "I'm not actually a file, I'll try to be as needed". let result = if let Some(h) = handle { drop(guard); - state.fs_rename(source_path, &host_adjusted_target_path) + let state = state; + __asyncify_light(env, None, async move { + state + .fs_rename(source_path, &host_adjusted_target_path) + .await + })? } else { let path_clone = path.clone(); drop(guard); - let out = state.fs_rename(path_clone, &host_adjusted_target_path); + let out = { + let state = state; + let host_adjusted_target_path = host_adjusted_target_path.clone(); + __asyncify_light(env, None, async move { + state + .fs_rename(path_clone, &host_adjusted_target_path) + .await + })? + }; { let mut guard = source_entry.write(); if let Kind::File { ref mut path, .. } = guard.deref_mut() { @@ -141,14 +154,23 @@ pub fn path_rename( let mut guard = source_parent_inode.write(); if let Kind::Dir { entries, .. } = guard.deref_mut() { entries.insert(source_entry_name, source_entry); - return e; + return Ok(e); } } } Kind::Dir { ref path, .. } => { let cloned_path = path.clone(); - if let Err(e) = state.fs_rename(cloned_path, &host_adjusted_target_path) { - return e; + let res = { + let state = state; + let host_adjusted_target_path = host_adjusted_target_path.clone(); + __asyncify_light(env, None, async move { + state + .fs_rename(cloned_path, &host_adjusted_target_path) + .await + })? + }; + if let Err(e) = res { + return Ok(e); } { drop(guard); @@ -178,5 +200,5 @@ pub fn path_rename( } } - Errno::Success + Ok(Errno::Success) } diff --git a/lib/wasix/src/syscalls/wasi/path_unlink_file.rs b/lib/wasix/src/syscalls/wasi/path_unlink_file.rs index 218b3993fc1..c9711c70728 100644 --- a/lib/wasix/src/syscalls/wasi/path_unlink_file.rs +++ b/lib/wasix/src/syscalls/wasi/path_unlink_file.rs @@ -16,15 +16,15 @@ pub fn path_unlink_file( fd: WasiFd, path: WasmPtr, path_len: M::Offset, -) -> Errno { +) -> Result { let env = ctx.data(); let (memory, mut state, inodes) = unsafe { env.get_memory_and_wasi_state_and_inodes(&ctx, 0) }; - let base_dir = wasi_try!(state.fs.get_fd(fd)); + let base_dir = wasi_try_ok!(state.fs.get_fd(fd)); if !base_dir.rights.contains(Rights::PATH_UNLINK_FILE) { - return Errno::Access; + return Ok(Errno::Access); } - let mut path_str = unsafe { get_input_str!(&memory, path, path_len) }; + let mut path_str = unsafe { get_input_str_ok!(&memory, path, path_len) }; Span::current().record("path", path_str.as_str()); // Convert relative paths into absolute paths @@ -32,8 +32,8 @@ pub fn path_unlink_file( path_str = ctx.data().state.fs.relative_path_to_absolute(path_str); } - let inode = wasi_try!(state.fs.get_inode_at_path(inodes, fd, &path_str, false)); - let (parent_inode, childs_name) = wasi_try!(state.fs.get_parent_inode_at_path( + let inode = wasi_try_ok!(state.fs.get_inode_at_path(inodes, fd, &path_str, false)); + let (parent_inode, childs_name) = wasi_try_ok!(state.fs.get_parent_inode_at_path( inodes, fd, std::path::Path::new(&path_str), @@ -46,13 +46,13 @@ pub fn path_unlink_file( Kind::Dir { ref mut entries, .. } => { - let removed_inode = wasi_try!(entries.remove(&childs_name).ok_or(Errno::Inval)); + let removed_inode = wasi_try_ok!(entries.remove(&childs_name).ok_or(Errno::Inval)); // TODO: make this a debug assert in the future assert!(inode.ino() == removed_inode.ino()); debug_assert!(inode.stat.read().unwrap().st_nlink > 0); removed_inode } - Kind::Root { .. } => return Errno::Access, + Kind::Root { .. } => return Ok(Errno::Access), _ => unreachable!( "Internal logic error in wasi::path_unlink_file, parent is not a directory" ), @@ -71,17 +71,22 @@ pub fn path_unlink_file( Kind::File { handle, path, .. } => { if let Some(h) = handle { let mut h = h.write().unwrap(); - wasi_try!(h.unlink().map_err(fs_error_into_wasi_err)); + let state = state; + let fut = h.unlink(); + drop(h); + wasi_try_ok!(__asyncify_light(env, None, async move { + fut.await.map_err(fs_error_into_wasi_err) + })?) } else { // File is closed // problem with the abstraction, we can't call unlink because there's no handle // drop mutable borrow on `path` let path = path.clone(); drop(guard); - wasi_try!(state.fs_remove_file(path)); + wasi_try_ok!(state.fs_remove_file(path)); } } - Kind::Dir { .. } | Kind::Root { .. } => return Errno::Isdir, + Kind::Dir { .. } | Kind::Root { .. } => return Ok(Errno::Isdir), Kind::Symlink { .. } => { // TODO: actually delete real symlinks and do nothing for virtual symlinks } @@ -90,5 +95,5 @@ pub fn path_unlink_file( } } - Errno::Success + Ok(Errno::Success) } diff --git a/tests/integration/cli/tests/run_unstable.rs b/tests/integration/cli/tests/run_unstable.rs index 8d418f3dad7..d889e7dfa7b 100644 --- a/tests/integration/cli/tests/run_unstable.rs +++ b/tests/integration/cli/tests/run_unstable.rs @@ -509,7 +509,10 @@ mod fixtures { /// A helper that wraps [`std::process::Child`] to make sure it gets terminated /// when it is no longer needed. -struct JoinableChild(Option); +struct JoinableChild { + command: std::process::Command, + child: Option, +} impl JoinableChild { fn spawn(mut cmd: std::process::Command) -> Self { @@ -520,14 +523,17 @@ impl JoinableChild { .spawn() .unwrap(); - JoinableChild(Some(child)) + JoinableChild { + child: Some(child), + command: cmd, + } } /// Keep reading lines from the child's stdout until a line containing the /// desired text is found. fn wait_for_stdout(&mut self, text: &str) -> String { let stdout = self - .0 + .child .as_mut() .and_then(|child| child.stdout.as_mut()) .unwrap(); @@ -539,7 +545,7 @@ impl JoinableChild { /// desired text is found. fn wait_for_stderr(&mut self, text: &str) -> String { let stderr = self - .0 + .child .as_mut() .and_then(|child| child.stderr.as_mut()) .unwrap(); @@ -550,7 +556,7 @@ impl JoinableChild { /// Kill the underlying [`std::process::Child`] and get an [`Assert`] we /// can use to check it. fn join(mut self) -> Assert { - let mut child = self.0.take().unwrap(); + let mut child = self.child.take().unwrap(); child.kill().unwrap(); child.wait_with_output().unwrap().assert() } @@ -597,8 +603,9 @@ fn read_line(reader: &mut dyn Read) -> Result { impl Drop for JoinableChild { fn drop(&mut self) { - if let Some(mut child) = self.0.take() { + if let Some(mut child) = self.child.take() { eprintln!("==== WARNING: Child was dropped before being joined ===="); + eprintln!("Command: {:?}", self.command); let _ = child.kill(); diff --git a/tests/lib/wast/Cargo.toml b/tests/lib/wast/Cargo.toml index fb94f5a1391..e098da653fb 100644 --- a/tests/lib/wast/Cargo.toml +++ b/tests/lib/wast/Cargo.toml @@ -19,6 +19,7 @@ wast = "38.0" serde = "1" tempfile = "3.4.0" thiserror = "1.0" +futures = "0.3" tokio = { version = "1", features = [ "io-util", "rt" ], default_features = false } [features] diff --git a/tests/lib/wast/src/wasi_wast.rs b/tests/lib/wast/src/wasi_wast.rs index af638143294..15a4dd23380 100644 --- a/tests/lib/wast/src/wasi_wast.rs +++ b/tests/lib/wast/src/wasi_wast.rs @@ -1,10 +1,14 @@ -use std::fs::{read_dir, File, OpenOptions, ReadDir}; -use std::future::Future; -use std::io::{self, Read, SeekFrom}; -use std::path::{Path, PathBuf}; -use std::pin::Pin; -use std::sync::{mpsc, Arc, Mutex}; -use std::task::{Context, Poll}; +use std::{ + fs::{read_dir, File, OpenOptions, ReadDir}, + future::Future, + io::{self, Read, SeekFrom}, + path::{Path, PathBuf}, + pin::Pin, + sync::{mpsc, Arc, Mutex}, + task::{Context, Poll}, +}; + +use futures::future::BoxFuture; use virtual_fs::{ host_fs, mem_fs, passthru_fs, tmp_fs, union_fs, AsyncRead, AsyncSeek, AsyncWrite, AsyncWriteExt, FileSystem, Pipe, ReadBuf, RootFileSystemBuilder, @@ -630,8 +634,8 @@ impl VirtualFile for OutputCapturerer { fn set_len(&mut self, _new_size: Filesize) -> Result<(), FsError> { Ok(()) } - fn unlink(&mut self) -> Result<(), FsError> { - Ok(()) + fn unlink(&mut self) -> BoxFuture<'static, Result<(), FsError>> { + Box::pin(async { Ok(()) }) } fn poll_read_ready(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { Poll::Ready(Ok(0))