Browse Source

preview1: Update semantics of seeking on appending files (#7586)

This commit updates the semantics of `fd_{seek,tell}` on preview1 to
match native Unix when used with appending files. On Unix `write` claims
to always update the file position pointer to the end of the file, so
this commit implements that instead of the previous logic of ignoring
the position update for appending files. This currently requires an
extra roundtrip via `stat` to figure out the size of the file, but for
now that seems to be the best that can be done.

Closes #7583
pull/7591/head
Alex Crichton 12 months ago
committed by GitHub
parent
commit
55ac2688d7
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 47
      crates/test-programs/src/bin/preview1_file_seek_tell.rs
  2. 16
      crates/wasi-preview1-component-adapter/src/lib.rs
  3. 6
      crates/wasi/src/preview2/preview1.rs

47
crates/test-programs/src/bin/preview1_file_seek_tell.rs

@ -79,6 +79,48 @@ unsafe fn test_file_seek_tell(dir_fd: wasi::Fd) {
wasi::fd_close(file_fd).expect("closing a file"); wasi::fd_close(file_fd).expect("closing a file");
wasi::path_unlink_file(dir_fd, "file").expect("deleting a file"); wasi::path_unlink_file(dir_fd, "file").expect("deleting a file");
} }
// Test that when a file is opened with `O_APPEND` that acquiring the current
// position indicates the end of the file.
unsafe fn seek_and_o_append(dir_fd: wasi::Fd) {
let path = "file2";
let file_fd = wasi::path_open(
dir_fd,
0,
path,
wasi::OFLAGS_CREAT,
wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
0,
wasi::FDFLAGS_APPEND,
)
.expect("opening a file");
assert!(
file_fd > libc::STDERR_FILENO as wasi::Fd,
"file descriptor range check",
);
let mut offset = wasi::fd_seek(file_fd, 0, wasi::WHENCE_CUR).unwrap();
assert_eq!(offset, 0);
offset = wasi::fd_tell(file_fd).unwrap();
assert_eq!(offset, 0);
let data = &[0u8; 100];
let iov = wasi::Ciovec {
buf: data.as_ptr() as *const _,
buf_len: data.len(),
};
let nwritten = wasi::fd_write(file_fd, &[iov]).unwrap();
assert_eq!(nwritten, 100);
let mut offset = wasi::fd_seek(file_fd, 0, wasi::WHENCE_CUR).unwrap();
assert_eq!(offset, 100);
offset = wasi::fd_tell(file_fd).unwrap();
assert_eq!(offset, 100);
wasi::fd_close(file_fd).unwrap();
wasi::path_unlink_file(dir_fd, path).unwrap();
}
fn main() { fn main() {
let mut args = env::args(); let mut args = env::args();
let prog = args.next().unwrap(); let prog = args.next().unwrap();
@ -99,5 +141,8 @@ fn main() {
}; };
// Run the tests. // Run the tests.
unsafe { test_file_seek_tell(dir_fd) } unsafe {
test_file_seek_tell(dir_fd);
seek_and_o_append(dir_fd);
}
} }

16
crates/wasi-preview1-component-adapter/src/lib.rs

@ -1293,12 +1293,18 @@ pub unsafe extern "C" fn fd_write(
BlockingMode::Blocking.write(wasi_stream, bytes)? BlockingMode::Blocking.write(wasi_stream, bytes)?
}; };
// If this is a file, keep the current-position pointer up to date. // If this is a file, keep the current-position pointer up
// to date. Note that for files that perform appending
// writes this function will always update the current
// position to the end of the file.
//
// NB: this isn't "atomic" as it doesn't necessarily account
// for concurrent writes, but there's not much that can be
// done about that.
if let StreamType::File(file) = &streams.type_ { if let StreamType::File(file) = &streams.type_ {
// But don't update if we're in append mode. Strictly speaking, if file.append {
// we should set the position to the new end of the file, but file.position.set(file.fd.stat()?.size);
// we don't have an API to do that atomically. } else {
if !file.append {
file.position.set(file.position.get() + nbytes as u64); file.position.set(file.position.get() + nbytes as u64);
} }
} }

6
crates/wasi/src/preview2/preview1.rs

@ -1428,6 +1428,7 @@ impl<
position, position,
}) if t.view.table().get(fd)?.is_file() => { }) if t.view.table().get(fd)?.is_file() => {
let fd = fd.borrowed(); let fd = fd.borrowed();
let fd2 = fd.borrowed();
let blocking_mode = *blocking_mode; let blocking_mode = *blocking_mode;
let position = position.clone(); let position = position.clone();
let append = *append; let append = *append;
@ -1452,7 +1453,10 @@ impl<
(stream, pos) (stream, pos)
}; };
let n = blocking_mode.write(self, stream, &buf).await?; let n = blocking_mode.write(self, stream, &buf).await?;
if !append { if append {
let len = self.stat(fd2).await?;
position.store(len.size, Ordering::Relaxed);
} else {
let pos = pos.checked_add(n as u64).ok_or(types::Errno::Overflow)?; let pos = pos.checked_add(n as u64).ok_or(types::Errno::Overflow)?;
position.store(pos, Ordering::Relaxed); position.store(pos, Ordering::Relaxed);
} }

Loading…
Cancel
Save