From 865c2faaf306539867e3165dfb8aa116aca11906 Mon Sep 17 00:00:00 2001 From: Yage Hu Date: Sun, 20 Oct 2024 00:01:00 -0400 Subject: [PATCH 1/2] Fix append-ness of new fd affecting old fd This commit fixes a case where opening a new file descriptor with the append flag makes previously opened non-append fds append as well. The fix is relatively simple, since the append-ness of an fd is already tracked in the virtual fs implementation, there's no need to open the actual file with the append flag. This behavior is consistent across Linux and other Wasm runtimes. fixes #5161 Signed-off-by: Yage Hu --- lib/wasix/src/syscalls/wasi/fd_write.rs | 6 +++- lib/wasix/src/syscalls/wasi/path_open.rs | 2 +- tests/wasi-fyi/fs_open_append_offset.rs | 31 +++++++++++++++++++ .../fyi/fs_open_append_offset.dir/.gitignore | 1 + 4 files changed, 38 insertions(+), 2 deletions(-) create mode 100644 tests/wasi-fyi/fs_open_append_offset.rs create mode 100644 tests/wasi-fyi/test_fs/fyi/fs_open_append_offset.dir/.gitignore diff --git a/lib/wasix/src/syscalls/wasi/fd_write.rs b/lib/wasix/src/syscalls/wasi/fd_write.rs index 04954d6770e..3ca198875ee 100644 --- a/lib/wasix/src/syscalls/wasi/fd_write.rs +++ b/lib/wasix/src/syscalls/wasi/fd_write.rs @@ -163,10 +163,12 @@ pub(crate) fn fd_write_internal( if !is_stdio { if fd_entry.flags.contains(Fdflags::APPEND) { // `fdflags::append` means we need to seek to the end before writing. - offset = handle.size(); + offset = fd_entry.inode.stat.read().unwrap().st_size; fd_entry.offset.store(offset, Ordering::Release); } + println!("whoa {offset}"); + handle .seek(std::io::SeekFrom::Start(offset)) .await @@ -206,6 +208,8 @@ pub(crate) fn fd_write_internal( } } + println!("written {written} {}", handle.size()); + if is_stdio { handle.flush().await.map_err(map_io_err)?; } diff --git a/lib/wasix/src/syscalls/wasi/path_open.rs b/lib/wasix/src/syscalls/wasi/path_open.rs index 9c8d3c81655..4f8c00d57dd 100644 --- a/lib/wasix/src/syscalls/wasi/path_open.rs +++ b/lib/wasix/src/syscalls/wasi/path_open.rs @@ -237,7 +237,7 @@ pub(crate) fn path_open_internal( let open_options = open_options .write(minimum_rights.write) .create(minimum_rights.create) - .append(minimum_rights.append) + .append(false) .truncate(minimum_rights.truncate); if minimum_rights.read { diff --git a/tests/wasi-fyi/fs_open_append_offset.rs b/tests/wasi-fyi/fs_open_append_offset.rs new file mode 100644 index 00000000000..fef83616f53 --- /dev/null +++ b/tests/wasi-fyi/fs_open_append_offset.rs @@ -0,0 +1,31 @@ +use std::{ + fs, + io::{Seek, SeekFrom, Write}, +}; + +fn main() { + let file = "fyi/fs_open_append_offset.dir/file"; + let mut f0 = fs::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(file) + .unwrap(); + + f0.write_all(b"abc").unwrap(); + f0.seek(SeekFrom::Start(1)).unwrap(); + + assert_eq!(fs::read_to_string(file).unwrap(), "abc"); + + // This open with append should not affect the offset of f0. + let _f1 = fs::OpenOptions::new() + .create(true) + .write(true) + .append(true) + .open(file) + .unwrap(); + + f0.write_all(b"d").unwrap(); + + assert_eq!(fs::read_to_string(file).unwrap(), "adc"); +} diff --git a/tests/wasi-fyi/test_fs/fyi/fs_open_append_offset.dir/.gitignore b/tests/wasi-fyi/test_fs/fyi/fs_open_append_offset.dir/.gitignore new file mode 100644 index 00000000000..f73f3093ff8 --- /dev/null +++ b/tests/wasi-fyi/test_fs/fyi/fs_open_append_offset.dir/.gitignore @@ -0,0 +1 @@ +file From f1377bef84612e66e9a2a091d0b1c935f3d9dc62 Mon Sep 17 00:00:00 2001 From: Yage Hu Date: Sun, 20 Oct 2024 00:29:38 -0400 Subject: [PATCH 2/2] Clean up prints Signed-off-by: Yage Hu --- lib/wasix/src/syscalls/wasi/fd_write.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/wasix/src/syscalls/wasi/fd_write.rs b/lib/wasix/src/syscalls/wasi/fd_write.rs index 3ca198875ee..7793b72a550 100644 --- a/lib/wasix/src/syscalls/wasi/fd_write.rs +++ b/lib/wasix/src/syscalls/wasi/fd_write.rs @@ -167,8 +167,6 @@ pub(crate) fn fd_write_internal( fd_entry.offset.store(offset, Ordering::Release); } - println!("whoa {offset}"); - handle .seek(std::io::SeekFrom::Start(offset)) .await @@ -208,8 +206,6 @@ pub(crate) fn fd_write_internal( } } - println!("written {written} {}", handle.size()); - if is_stdio { handle.flush().await.map_err(map_io_err)?; }