nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
108.03k stars 29.82k forks source link

WASI preview 1: `fd_readdir` ignores `cookie` argument #47193

Open uhyo opened 1 year ago

uhyo commented 1 year ago

Version

v19.8.1

Platform

Darwin uhyo 21.6.0 Darwin Kernel Version 21.6.0: Mon Aug 22 20:19:52 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

Create a WASM binary that calls the fd_readdir function with a non-zero cookie argument.

Reproduction repository: https://github.com/uhyo/rust-wasi-readdir

Example Rust code that demonstrates the problem ```rust use std::{env, fs, io}; fn main() -> io::Result<()> { println!("Calling fs::read_dir"); for entry in fs::read_dir(env::current_dir()?)? { let entry = entry?; println!("Entry {}", entry.path().display()); } Ok(()) } ```

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

A typical use case of the fd_readdir function is to repeatedly call it until the caller can retrieve all entries in a directory. The cookie parameter is used as a cursor so that the function can know where in the given directory to continue from. The fd_readdir should respect the cookie parameter. (Read more in the reproduction repository's README)

What do you see instead?

Actually, the fd_readdir function ignores the cookie parameters and always returns the same set of entries. The caller falls into an infinite loop.

Log of `fd_readdir` calls and how Node.js responded to them ``` fd_readdir(4,1065824,128,0,1047548) = 0 offset = 0 { d_next: 0n, d_ino: 0n, d_namlen: 17, d_type: 4, name: 'run-node-wasi.mjs' } offset = 41 { d_next: 1n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.toml' } offset = 75 { d_next: 2n, d_ino: 0n, d_namlen: 20, d_type: 4, name: 'run-wasmtime-wasi.sh' } (offset = 119) Entry /run-node-wasi.mjs Entry /Cargo.toml Entry /run-wasmtime-wasi.sh fd_readdir(4,1065824,128,2,1047548) = 0 offset = 0 { d_next: 0n, d_ino: 0n, d_namlen: 17, d_type: 4, name: 'run-node-wasi.mjs' } offset = 41 { d_next: 1n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.toml' } offset = 75 { d_next: 2n, d_ino: 0n, d_namlen: 20, d_type: 4, name: 'run-wasmtime-wasi.sh' } (offset = 119) Entry /run-node-wasi.mjs Entry /Cargo.toml Entry /run-wasmtime-wasi.sh fd_readdir(4,1065824,128,2,1047548) = 0 offset = 0 { d_next: 0n, d_ino: 0n, d_namlen: 17, d_type: 4, name: 'run-node-wasi.mjs' } offset = 41 { d_next: 1n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.toml' } offset = 75 { d_next: 2n, d_ino: 0n, d_namlen: 20, d_type: 4, name: 'run-wasmtime-wasi.sh' } (offset = 119) Entry /run-node-wasi.mjs Entry /Cargo.toml Entry /run-wasmtime-wasi.sh fd_readdir(4,1065824,128,2,1047548) = 0 offset = 0 { d_next: 0n, d_ino: 0n, d_namlen: 17, d_type: 4, name: 'run-node-wasi.mjs' } offset = 41 { d_next: 1n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.toml' } offset = 75 { d_next: 2n, d_ino: 0n, d_namlen: 20, d_type: 4, name: 'run-wasmtime-wasi.sh' } (offset = 119) Entry /run-node-wasi.mjs Entry /Cargo.toml Entry /run-wasmtime-wasi.sh fd_readdir(4,1065824,128,2,1047548) = 0 offset = 0 { d_next: 0n, d_ino: 0n, d_namlen: 17, d_type: 4, name: 'run-node-wasi.mjs' } offset = 41 { d_next: 1n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.toml' } offset = 75 { d_next: 2n, d_ino: 0n, d_namlen: 20, d_type: 4, name: 'run-wasmtime-wasi.sh' } (offset = 119) ```

Additional information

No response

mhdawson commented 1 year ago

Trying to follow the recreate following the instructions on what is in the main branch for Node.js I get:

root@cff172256677:~/rust-wasi-readdir# ../node/node --experimental-wasi-unstable-preview1 run-node-wasi.mjs 
(node:88306) ExperimentalWarning: WASI is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
Calling fs::read_dir
fd_readdir(4,1065088,128,0,1047548) = 0
offset = 0 { d_next: 14n, d_ino: 0n, d_namlen: 4, d_type: 3, name: '.git' }
offset = 28 { d_next: 17n, d_ino: 0n, d_namlen: 10, d_type: 4, name: '.gitignore' }
offset = 62 { d_next: 20n, d_ino: 0n, d_namlen: 10, d_type: 4, name: 'Cargo.lock' }
file:///root/rust-wasi-readdir/run-node-wasi.mjs:30
    const name = decoder.decode(new Uint8Array(resultBuf, offset + 24, dirent.d_namlen));
                                ^

RangeError: Invalid typed array length: 10
    at new Uint8Array (<anonymous>)
    at wasi.wasiImport.fd_readdir (file:///root/rust-wasi-readdir/run-node-wasi.mjs:30:33)
    at wasi::lib_generated::fd_readdir::h99e93ea1726a60a1 (wasm://wasm/008d5366:wasm-function[212]:0x12dd4)
    at <std::sys::wasi::fs::ReadDir as core::iter::traits::iterator::Iterator>::next::h78f71fa31b6d237f (wasm://wasm/008d5366:wasm-function[24]:0x6858)
    at <std::fs::ReadDir as core::iter::traits::iterator::Iterator>::next::h15f0dfa6aec99ed5 (wasm://wasm/008d5366:wasm-function[106]:0xef8e)
    at rust_wasi_readdir::main::hd7fee8bf240a7350 (wasm://wasm/008d5366:wasm-function[10]:0x254d)
    at core::ops::function::FnOnce::call_once::h6cf85a7bce0f2d1a (wasm://wasm/008d5366:wasm-function[245]:0x1370f)
    at std::sys_common::backtrace::__rust_begin_short_backtrace::h389a6674b2acf4bb (wasm://wasm/008d5366:wasm-function[244]:0x136d9)
    at std::rt::lang_start::{{closure}}::haccc3febcb60cb5f (wasm://wasm/008d5366:wasm-function[154]:0x1156e)
    at std::rt::lang_start_internal::h38aaea5d7881ae71 (wasm://wasm/008d5366:wasm-function[40]:0x9383)

Node.js v20.0.0-pre

If I comment out the instumentaion that seems to be failing I get:

oot@cff172256677:~/rust-wasi-readdir# ../node/node --experimental-wasi-unstable-preview1 run-node-wasi.mjs 
(node:88318) ExperimentalWarning: WASI is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
Calling fs::read_dir
Entry /.git
Entry /.gitignore
Entry /Cargo.lock
Entry /Cargo.toml
Entry /README.md
Entry /run-deno-wasi.ts
Entry /run-wasmtime-wasi.sh
Entry /src
Entry /target
Entry /run-node-wasi.mjs

This is on ubuntu x86.

mhdawson commented 1 year ago

Same results for me if I use 19.8.1 on ubuntu x86 as well:

root@cff172256677:~/rust-wasi-readdir# /root/node-v19.8.1-linux-x64/bin/node --experimental-wasi-unstable-preview1 run-node-wasi.mjs 
(node:88361) ExperimentalWarning: WASI is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
Calling fs::read_dir
Entry /.git
Entry /.gitignore
Entry /Cargo.lock
Entry /Cargo.toml
Entry /README.md
Entry /run-deno-wasi.ts
Entry /run-wasmtime-wasi.sh
Entry /src
Entry /target
Entry /run-node-wasi.mjs
mhdawson commented 1 year ago

Unfortunately I don't have an ARM OSX machine where docker etc can be installed to test there. @uhyo have you recreated on any other platform/OS combinations?

uhyo commented 1 year ago

Sorry, I haven't tested on any other platforms. It's not surprising if it is specific to Mac.

uhyo commented 1 year ago

It might be specific to a file system type, so here is additional information:

diskutil info /dev/disk3s1 ``` Device Identifier: disk3s1 Device Node: /dev/disk3s1 Whole: No Part of Whole: disk3 Volume Name: Macintosh HD - Data Mounted: Yes Mount Point: /System/Volumes/Data Partition Type: 41504653-0000-11AA-AA11-00306543ECAC File System Personality: APFS Type (Bundle): apfs Name (User Visible): APFS Owners: Enabled OS Can Be Installed: Yes Booter Disk: disk3s4 Recovery Disk: disk3s5 Media Type: Generic Protocol: Apple Fabric SMART Status: Verified Volume UUID: BAB71912-9E0C-4972-8CD4-38F17A98CCD3 Disk / Partition UUID: BAB71912-9E0C-4972-8CD4-38F17A98CCD3 Disk Size: 494.4 GB (494384795648 Bytes) (exactly 965595304 512-Byte-Units) Device Block Size: 4096 Bytes Container Total Space: 494.4 GB (494384795648 Bytes) (exactly 965595304 512-Byte-Units) Container Free Space: 340.1 GB (340057763840 Bytes) (exactly 664175320 512-Byte-Units) Allocation Block Size: 4096 Bytes Media OS Use Only: No Media Read-Only: No Volume Read-Only: No Device Location: Internal Removable Media: Fixed Solid State: Yes Hardware AES Support: Yes This disk is an APFS Volume. APFS Information: APFS Container: disk3 APFS Physical Store: disk0s2 Fusion Drive: No APFS Volume Group: 8D5D901F-196A-4E7A-B079-C163D9585F6D FileVault: No Sealed: No Locked: No ```
mhdawson commented 1 year ago

We discussed in the uvwasi team meeting yesterday and @cjihrig is going to take a look as he as an M1 mac.

gip commented 4 weeks ago

Basically the example from https://doc.rust-lang.org/std/fs/struct.DirEntry.html loops forever (going through the same 3 files out of ~20 in the directory) with node v20.11.1 and the wasi preview1 target on macOS. Adding that here for future reference in case more people run into this.

use std::fs;

if let Ok(entries) = fs::read_dir(".") {
    for entry in entries {
        if let Ok(entry) = entry {
            // Here, `entry` is a `DirEntry`.
            if let Ok(metadata) = entry.metadata() {
                // Now let's show our entry's permissions!
                println!("{:?}: {:?}", entry.path(), metadata.permissions());
            } else {
                println!("Couldn't get metadata for {:?}", entry.path());
            }
        }
    }
}