bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.1k stars 1.26k forks source link

file truncation with preview2 adapter? #7390

Closed yamt closed 10 months ago

yamt commented 10 months ago

i have a rust program which uses the image crate (0.24.7 if it matters) to write png/jpg files to filesystem. i build it for the wasm32-wasi target, and adapt it with the preview2 adapter, and run it on my wasmtime-based embedder.

while small image files are generated fine, it seems larger files (> 4096 bytes?) are somehow corrupted.

i'm using wasmtime 14.0.2.

Test Case

unfortunately i can't share my programs.

Steps to Reproduce

see above.

Expected Results

see above.

Actual Results

see above.

Versions and Environment

Wasmtime version or commit: 14.0.2

Operating system: macOS

Architecture: amd64

Extra Info

tschneidereit commented 10 months ago

One thing I could imagine happening is that your code sends a fd_write that used to be executed in full, and with the adapter only the first 4KB are written. This is a bit of a common pitfall with POSIX streams in general (see the description of the write syscall), where there's implementation defined behavior for how much to write. To ensure that everything is written, you have to call fd_write (or e.g. fwrite in C) in a loop, checking how many bytes were actually written.

If that's not the issue you're running into, we'd probably need a bit more information to debug, perhaps even a reduced test case.

yamt commented 10 months ago

One thing I could imagine happening is that your code sends a fd_write that used to be executed in full, and with the adapter only the first 4KB are written. This is a bit of a common pitfall with POSIX streams in general (see the description of the write syscall), where there's implementation defined behavior for how much to write. To ensure that everything is written, you have to call fd_write (or e.g. fwrite in C) in a loop, checking how many bytes were actually written.

this is a regular file. partial success should be really exceptional. (like ENOSPC)

bjorn3 commented 10 months ago

On Unix partial success can also happen because of a signal (even just SIGSTOP for ctrl-z). In case of the preview2 adapter partial success will always happen when attempting to write more than 4KB. It has an internal buffer of 4KB: https://github.com/bytecodealliance/wasmtime/blob/fa6fcd946b8f6d60c2d191a1b14b9399e261a76d/crates/wasi/src/preview2/preview1.rs#L92-L93

yamt commented 10 months ago

On Unix partial success can also happen because of a signal (even just SIGSTOP for ctrl-z).

is there any real platform where regular file i/o is interruptible by signals? only things i'm aware of is some of NFS interruptible mount implementations. (and of course NFS is not a posix-compatible filesystem.)

In case of the preview2 adapter partial success will always happen when attempting to write more than 4KB. It has an internal buffer of 4KB:

https://github.com/bytecodealliance/wasmtime/blob/fa6fcd946b8f6d60c2d191a1b14b9399e261a76d/crates/wasi/src/preview2/preview1.rs#L92-L93

it sounds like a bug in the adapter.

bjorn3 commented 10 months ago

Found this in the write(2) man page:

On Linux, write() (and similar system calls) will transfer at most 0x7ffff000 (2,147,479,552) bytes, returning the number of bytes actually transferred. (This is true on both 32-bit and 64-bit systems.)

yamt commented 10 months ago

Found this in the write(2) man page:

On Linux, write() (and similar system calls) will transfer at most 0x7ffff000 (2,147,479,552) bytes, returning the number of bytes actually transferred. (This is true on both 32-bit and 64-bit systems.)

i don't complain about 0x7ffff000 bytes. but i do about 4096 bytes.

the point here is that many of apps written for unix assume partial success of i/o on regular files is exceptional. while you might find a strict interpretation of posix allows the behavior (i dunno) it breaks real programs out there.

bjorn3 commented 10 months ago

i don't complain about 0x7ffff000 bytes. but i do about 4096 bytes.

It doesn't matter at which point the cutoff is. A program which doesn't correctly handle partial success can hit it. In rust we have io::Write::write_all for when you actually want to write everything and use of it over io::Write::write is encouraged.

is there any real platform where regular file i/o is interruptible by signals?

Linux if you disable SA_RESTART I believe.

tschneidereit commented 10 months ago

Looking at the adapter code, it seems like for file descriptors without the nonblocking flag, the full contents should be written.

@yamt can you confirm that you're not operating on an FD with FDFLAGS_NONBLOCK set for some reason?

pchickey commented 10 months ago

I have reproduced this and am working on a fix.

yamt commented 10 months ago

i don't complain about 0x7ffff000 bytes. but i do about 4096 bytes.

It doesn't matter at which point the cutoff is.

in real world, it matters.

A program which doesn't correctly handle partial success can hit it. In rust we have io::Write::write_all for when you actually want to write everything and use of it over io::Write::write is encouraged.

for some file types like stream-type-socket, it's a common way to handle partial success. for other types including regular files, it depends on applications how partial success should be handled. note that posix somehow guarantees i/o atomicity among threads and there are applications relies on the behaviour.

is there any real platform where regular file i/o is interruptible by signals?

Linux if you disable SA_RESTART I believe.

i'm sure it was not the case when i was working on linux. (more than a decade ago) i can't believe things like this have been changed since then. but i don't know.

yamt commented 10 months ago

Looking at the adapter code, it seems like for file descriptors without the nonblocking flag, the full contents should be written.

@yamt can you confirm that you're not operating on an FD with FDFLAGS_NONBLOCK set for some reason?

sorry, it isn't an easy task for me because i'm a rust newbie and the program in question is written in rust. it basically calls this save function to create a file.

yamt commented 10 months ago

I have reproduced this and am working on a fix.

thank you!

pchickey commented 10 months ago

Thank you @yamt for the bug report, I am sorry we didn't have coverage for this pretty basic problem in our test suite but we do now.

yamt commented 10 months ago

Thank you @yamt for the bug report, I am sorry we didn't have coverage for this pretty basic problem in our test suite but we do now.

thank you for the quick fix. my app is now working fine with wasmtime 14.0.4.