diff --git a/crates/perry-runtime/src/fs/callbacks.rs b/crates/perry-runtime/src/fs/callbacks.rs index d2ad96c36..f07397e33 100644 --- a/crates/perry-runtime/src/fs/callbacks.rs +++ b/crates/perry-runtime/src/fs/callbacks.rs @@ -982,13 +982,21 @@ pub extern "C" fn js_fs_read_callback( crate::closure::js_closure_call3(cb, err_val, 0.0, buffer_value); return f64::from_bits(TAG_UNDEFINED); } - let bytes = js_fs_read_sync( + let bytes = match crate::fs::read_sync_result( fd_value, buffer_value, offset_value, length_value, position_value, - ); + ) { + Ok(bytes) => bytes, + // The callback form reports the syscall error, it does not throw. + Err(err) => { + let err_val = unsafe { build_fs_error_value_no_path(&err, "read") }; + crate::closure::js_closure_call3(cb, err_val, 0.0, buffer_value); + return f64::from_bits(TAG_UNDEFINED); + } + }; if !cb.is_null() { crate::closure::js_closure_call3(cb, f64::from_bits(TAG_NULL), bytes, buffer_value); } @@ -1017,7 +1025,16 @@ pub extern "C" fn js_fs_read_callback_options( .unwrap_or_else(|| (buffer_len - offset).max(0.0)); let position = unsafe { options_number_field(options_value, b"position") } .unwrap_or(f64::from_bits(crate::value::TAG_NULL)); - let bytes = js_fs_read_sync(fd_value, buffer_value, offset, length, position); + let bytes = match crate::fs::read_sync_result(fd_value, buffer_value, offset, length, position) + { + Ok(bytes) => bytes, + // The callback form reports the syscall error, it does not throw. + Err(err) => { + let err_val = unsafe { build_fs_error_value_no_path(&err, "read") }; + crate::closure::js_closure_call3(cb, err_val, 0.0, buffer_value); + return f64::from_bits(TAG_UNDEFINED); + } + }; if !cb.is_null() { crate::closure::js_closure_call3(cb, f64::from_bits(TAG_NULL), bytes, buffer_value); } diff --git a/crates/perry-runtime/src/fs/fd_ops.rs b/crates/perry-runtime/src/fs/fd_ops.rs index 16ab08194..f1243b52f 100644 --- a/crates/perry-runtime/src/fs/fd_ops.rs +++ b/crates/perry-runtime/src/fs/fd_ops.rs @@ -68,6 +68,7 @@ fn open_options_from_flags(flags_value: f64) -> (fs::OpenOptions, bool) { #[cfg(unix)] fn apply_numeric_open_flags(opts: &mut fs::OpenOptions, flags: i32) -> bool { + use std::os::unix::fs::OpenOptionsExt; match flags & libc::O_ACCMODE { libc::O_WRONLY => { opts.write(true); @@ -91,6 +92,14 @@ fn apply_numeric_open_flags(opts: &mut fs::OpenOptions, flags: i32) -> bool { if append_mode { opts.append(true).write(true); } + // Everything `OpenOptions` does not model — O_NONBLOCK, O_NOCTTY, O_NOFOLLOW, + // O_SYNC, O_CLOEXEC… — still has to reach open(2); Node passes the flag word + // through verbatim. Dropping O_NONBLOCK left the descriptor blocking, so a + // `readSync` that Node answers with EAGAIN hung forever instead. + // `custom_flags` cannot clobber the access mode: std masks O_ACCMODE out. + const MODELED: i32 = + libc::O_ACCMODE | libc::O_CREAT | libc::O_EXCL | libc::O_TRUNC | libc::O_APPEND; + opts.custom_flags(flags & !MODELED); append_mode } @@ -199,6 +208,33 @@ pub extern "C" fn js_fs_read_sync( position_value: f64, ) -> f64 { crate::fs::validate::validate_fd_open(fd_value, "read"); + match read_sync_result( + fd_value, + buffer_value, + offset_value, + length_value, + position_value, + ) { + Ok(bytes) => bytes, + // Node surfaces the syscall error (EAGAIN on a non-blocking fd with no + // data ready, EISDIR, EIO…). Collapsing every failure to 0 made it + // indistinguishable from a clean EOF. + Err(err) => unsafe { + crate::exception::js_throw(build_fs_error_value_no_path(&err, "read")) + }, + } +} + +/// The `readSync` core, with the syscall error preserved so the throwing +/// (`fs.readSync`) and reporting (`fs.read(…, cb)`) surfaces can each render it +/// the way Node does. +pub(crate) fn read_sync_result( + fd_value: f64, + buffer_value: f64, + offset_value: f64, + length_value: f64, + position_value: f64, +) -> Result { let fd = fd_value as i32; let offset = offset_value.max(0.0) as usize; let length = length_value.max(0.0) as usize; @@ -209,12 +245,12 @@ pub extern "C" fn js_fs_read_sync( }; let buf = buffer_ptr_from_value(buffer_value); if buf.is_null() { - return 0.0; + return Ok(0.0); } FD_REGISTRY.with(|r| { let mut reg = r.borrow_mut(); let Some(file) = reg.get_mut(&fd) else { - return 0.0; + return Ok(0.0); }; let restore_pos = position.and_then(|_| file.stream_position().ok()); if let Some(pos) = position { @@ -226,13 +262,13 @@ pub extern "C" fn js_fs_read_sync( if let Some(pos) = restore_pos { let _ = file.seek(SeekFrom::Start(pos)); } - return 0.0; + return Ok(0.0); } let n = length.min(cap - offset); let data = crate::buffer::buffer_data_mut(buf).add(offset); let result = match file.read(std::slice::from_raw_parts_mut(data, n)) { - Ok(read) => read as f64, - Err(_) => 0.0, + Ok(read) => Ok(read as f64), + Err(err) => Err(err), }; if let Some(pos) = restore_pos { let _ = file.seek(SeekFrom::Start(pos)); diff --git a/tests/issue_fs_nonblocking_read.js b/tests/issue_fs_nonblocking_read.js new file mode 100644 index 000000000..d75b0b508 --- /dev/null +++ b/tests/issue_fs_nonblocking_read.js @@ -0,0 +1,49 @@ +// `openSync` must pass the flags it does not model through to open(2), and a +// failing `readSync` must surface the syscall error instead of reporting EOF. +// +// Perry translated only O_ACCMODE/O_CREAT/O_EXCL/O_TRUNC/O_APPEND into Rust's +// OpenOptions and dropped the rest, so O_NONBLOCK never reached open(2): the +// descriptor stayed blocking and a `readSync` with no data available hung +// forever where Node throws EAGAIN. `readSync` also mapped every error to 0, +// making a failure indistinguishable from a clean end-of-file. + +import { openSync, readSync, closeSync, constants } from "fs"; + +// A FIFO has no writer, so a non-blocking read has nothing to deliver: Node +// answers EAGAIN. (A blocking descriptor would park here forever.) +import { mkdtempSync, rmSync } from "fs"; +import { execFileSync } from "child_process"; +import { tmpdir } from "os"; +import { join } from "path"; + +const dir = mkdtempSync(join(tmpdir(), "perry-nonblock-")); + +// The fifo and the descriptor must be released even when an assertion below +// throws, or a failing run leaves a temp directory (and an open fd) behind on +// every retry. +try { + const fifo = join(dir, "fifo"); + execFileSync("mkfifo", [fifo]); + + const fd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + try { + const buf = Buffer.alloc(16); + let code = null; + try { + readSync(fd, buf, 0, 16, null); + throw new Error("readSync resolved instead of raising EAGAIN"); + } catch (err) { + code = err.code; + } + + if (code !== "EAGAIN") { + throw new Error(`readSync on an empty non-blocking FIFO reported ${code}, expected EAGAIN`); + } + } finally { + closeSync(fd); + } +} finally { + rmSync(dir, { recursive: true, force: true }); +} + +console.log("nonblocking-read ok"); diff --git a/tests/test_fs_nonblocking_read.sh b/tests/test_fs_nonblocking_read.sh new file mode 100755 index 000000000..82958ceac --- /dev/null +++ b/tests/test_fs_nonblocking_read.sh @@ -0,0 +1,62 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PERRY="${PERRY_BIN:-${PERRY:-$ROOT/target/release/perry}}" +RUNTIME_DIR="${PERRY_RUNTIME_DIR:-$ROOT/target/release}" +FIXTURE="$ROOT/tests/issue_fs_nonblocking_read.js" +WORKDIR="${TMPDIR:-/tmp}/perry-fs-nonblocking-read-$$" +BIN="$WORKDIR/perry-fs-nonblocking-read" +COMPILE_LOG="$WORKDIR/compile.log" +STDOUT_LOG="$WORKDIR/stdout.log" +STDERR_LOG="$WORKDIR/stderr.log" + +mkdir -p "$WORKDIR" +trap 'rm -rf "$WORKDIR"' EXIT + +if [[ ! -x "$PERRY" ]]; then + PERRY="$ROOT/target/debug/perry" +fi +if [[ ! -x "$PERRY" ]]; then + echo "SKIP: perry binary not found (build with cargo build --release -p perry)" + exit 0 +fi +if ! command -v mkfifo >/dev/null 2>&1; then + echo "SKIP: mkfifo not available" + exit 0 +fi + +env PERRY_ALLOW_UNIMPLEMENTED=1 PERRY_RUNTIME_DIR="$RUNTIME_DIR" "$PERRY" compile --no-cache --no-auto-optimize "$FIXTURE" -o "$BIN" \ + >"$COMPILE_LOG" 2>&1 || { + cat "$COMPILE_LOG" >&2 + exit 1 + } + +# A blocking descriptor parks in readSync forever — cap the run so the +# regression shows up as a failure rather than a hung suite. +set +e +if command -v timeout >/dev/null 2>&1; then + timeout 20 "$BIN" >"$STDOUT_LOG" 2>"$STDERR_LOG" /dev/null 2>&1; then + gtimeout 20 "$BIN" >"$STDOUT_LOG" 2>"$STDERR_LOG" "$STDOUT_LOG" 2>"$STDERR_LOG" &2 + exit 1 +fi +if [[ "$run_rc" -ne 0 ]]; then + echo "Perry non-blocking read fixture failed with exit code $run_rc" >&2 + cat "$STDOUT_LOG" "$STDERR_LOG" >&2 + exit 1 +fi + +grep -qx "nonblocking-read ok" "$STDOUT_LOG" || { + echo "unexpected output:" >&2 + cat "$STDOUT_LOG" >&2 + exit 1 +}