From 171698dd58c069a66a243d486cbba815edcaa66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 14 Jul 2026 15:33:02 +0200 Subject: [PATCH 1/3] fix(fs): openSync dropped O_NONBLOCK, and readSync reported errors as EOF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects that together make a non-blocking read impossible. `apply_numeric_open_flags` translated only O_ACCMODE/O_CREAT/O_EXCL/O_TRUNC/ O_APPEND into Rust's `OpenOptions` and silently discarded everything else, so O_NONBLOCK never reached open(2). The descriptor came back blocking and const fd = openSync("/dev/tty", O_RDONLY | O_NONBLOCK); readSync(fd, buf, 0, 1024, null); // Node: throws EAGAIN parked forever instead of raising EAGAIN. Forward the flags `OpenOptions` does not model via `custom_flags` — Node passes the flag word to open(2) verbatim. (std masks O_ACCMODE out of custom flags, so the access mode cannot be clobbered.) `js_fs_read_sync` then mapped every read error to `0.0`, which a caller cannot distinguish from a clean end-of-file. Thread the `io::Error` out of the read core so the sync surface throws the Node-shaped error (EAGAIN, EISDIR, EIO…) and the callback surface reports it as `cb(err)` — the callback form must not throw. --- crates/perry-runtime/src/fs/callbacks.rs | 24 +++++++-- crates/perry-runtime/src/fs/fd_ops.rs | 46 ++++++++++++++++-- tests/issue_fs_nonblocking_read.js | 39 +++++++++++++++ tests/test_fs_nonblocking_read.sh | 62 ++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 8 deletions(-) create mode 100644 tests/issue_fs_nonblocking_read.js create mode 100755 tests/test_fs_nonblocking_read.sh diff --git a/crates/perry-runtime/src/fs/callbacks.rs b/crates/perry-runtime/src/fs/callbacks.rs index d2ad96c369..73fc6d6bbf 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,17 @@ 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 16ab08194e..f1243b52f8 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 0000000000..d7d60c2c65 --- /dev/null +++ b/tests/issue_fs_nonblocking_read.js @@ -0,0 +1,39 @@ +// `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 } from "fs"; +import { execFileSync } from "child_process"; +import { tmpdir } from "os"; +import { join } from "path"; + +const dir = mkdtempSync(join(tmpdir(), "perry-nonblock-")); +const fifo = join(dir, "fifo"); +execFileSync("mkfifo", [fifo]); + +const fd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); + +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`); +} + +closeSync(fd); +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 0000000000..82958ceac8 --- /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 +} From e7b251b7dbdaab418c1d86a82167e24637909d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 14 Jul 2026 19:42:22 +0200 Subject: [PATCH 2/3] style: cargo fmt --- crates/perry-runtime/src/fs/callbacks.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/fs/callbacks.rs b/crates/perry-runtime/src/fs/callbacks.rs index 73fc6d6bbf..f07397e331 100644 --- a/crates/perry-runtime/src/fs/callbacks.rs +++ b/crates/perry-runtime/src/fs/callbacks.rs @@ -1025,17 +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 = - 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); - } - }; + 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); } From 2f0fb5d8dbd33028383ddbec874fb2dbbee8a45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Tue, 14 Jul 2026 19:57:46 +0200 Subject: [PATCH 3/3] test(fs): release the fifo and temp dir even when the assertion fails The fixture created a mkdtemp directory and an fd and freed neither on the throwing path, so a failing run leaked a temp directory (and an open descriptor) on every retry. Wrap both in `finally`. --- tests/issue_fs_nonblocking_read.js | 40 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/tests/issue_fs_nonblocking_read.js b/tests/issue_fs_nonblocking_read.js index d7d60c2c65..d75b0b508d 100644 --- a/tests/issue_fs_nonblocking_read.js +++ b/tests/issue_fs_nonblocking_read.js @@ -11,29 +11,39 @@ 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 } from "fs"; +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-")); -const fifo = join(dir, "fifo"); -execFileSync("mkfifo", [fifo]); -const fd = openSync(fifo, constants.O_RDONLY | constants.O_NONBLOCK); - -const buf = Buffer.alloc(16); -let code = null; +// 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 { - readSync(fd, buf, 0, 16, null); - throw new Error("readSync resolved instead of raising EAGAIN"); -} catch (err) { - code = err.code; -} + 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`); + 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 }); } -closeSync(fd); console.log("nonblocking-read ok");