fix(ext/node): fix EEXIST error and data loss in writeFileSync on Windows by bartlomieju · Pull Request #33413 · denoland/deno

bartlomieju · 2026-04-24T06:43:25Z

Summary

Fixes three interacting bugs that cause writeFileSync to throw a misleading EEXIST error and corrupt files to 0 bytes on Windows after long-running operation (hours):

CRT fd leak in PipeWrap::close: The close method only cleaned up FdTable entries on #[cfg(unix)]. On Windows, each child process spawn with piped stdio leaked CRT file descriptors created by open_osfhandle. Over hours, the CRT fd table (limit ~8192) fills up.
Stale GetLastError() in raw_fd_for_file: When open_osfhandle fails (CRT table full), the error handler called last_os_error() which reads GetLastError(). But open_osfhandle is a CRT function that sets errno, not GetLastError(). The stale value from the prior CreateFileW(CREATE_ALWAYS) was ERROR_ALREADY_EXISTS (183), misreported as EEXIST. Same bug existed in child_stdio_to_fd and extra pipe fd creation in ext/process/lib.rs.
Data loss from premature truncation: CREATE_ALWAYS truncates existing files to 0 bytes before open_osfhandle runs. If open_osfhandle fails, the write never happens and the file stays at 0 bytes — permanent data loss. Fixed by deferring truncation: open with OPEN_ALWAYS, create the CRT fd, then truncate.

How the bugs interact

App spawns child processes with piped stdio → CRT fds leak (3 per spawn)
After hours, CRT fd table fills → open_osfhandle fails in raw_fd_for_file
Error reads stale GetLastError() = ERROR_ALREADY_EXISTS → EEXIST reported
But CreateFileW(CREATE_ALWAYS) already truncated the file → 0-byte file, data loss

This matches all reported symptoms:

EEXIST after hours of running
File becomes 0 bytes
After deleting file: UNKNOWN error (stale GetLastError() = 0) + new 0-byte file

Closes #33405

Changes

File	Change
`ext/node/ops/fs.rs`	Fix stale `GetLastError()` in `raw_fd_for_file`; defer truncation on Windows until CRT fd is created
`ext/node/ops/pipe_wrap.rs`	Add Windows handling to `close()`: remove FdTable entry + free CRT fd via `libc::close`
`ext/node/ops/stream_wrap.rs`	Add `get_fd()` getter on `LibUvStreamWrap` for PipeWrap close
`ext/process/lib.rs`	Fix same stale `GetLastError()` bug in `child_stdio_to_fd` macro and extra pipe fd path
`tests/unit_node/fs_test.ts`	Add test for repeated writeFileSync overwrites

Test plan

Existing writeFileSync overwrite test passes
Existing wx flag tests pass
New repeated-overwrite test passes
child_process tests pass
Windows CI (cannot test CRT fd exhaustion locally on macOS, but the fix is straightforward and cfg-gated)

…dows Fixes three interacting bugs that cause writeFileSync to throw a misleading EEXIST error and corrupt files to 0 bytes on Windows after long-running operation: 1. CRT fd leak in PipeWrap::close: The close method only cleaned up FdTable entries on Unix. On Windows, each child process spawn with piped stdio leaked CRT file descriptors via open_osfhandle. Over hours, the CRT fd table fills up. 2. Stale GetLastError in raw_fd_for_file: When open_osfhandle fails (CRT table full), the error handler called last_os_error() which reads GetLastError(). But open_osfhandle is a CRT function that sets errno, not GetLastError(). The stale value from the prior CreateFileW(CREATE_ALWAYS) was ERROR_ALREADY_EXISTS (183), misreported as EEXIST. 3. Data loss from premature truncation: CREATE_ALWAYS truncates existing files to 0 bytes before open_osfhandle runs. If open_osfhandle fails, the write never happens and the file stays at 0 bytes — permanent data loss. Fixes #33405

miracatbot

I think there’s still one issue here.

In three places, when _open_osfhandle() fails, the patch now hardcodes std::io::Error::from_raw_os_error(4) (ERROR_TOO_MANY_OPEN_FILES).
That fixes the stale-GetLastError() problem, but it also throws away the actual CRT failure reason from _open_osfhandle(), which reports via errno, not Win32 last-error.
So if _open_osfhandle() fails for some reason other than CRT-fd exhaustion, this will now always misreport the error as “too many open files”.

Concretely affected sites:

ext/node/ops/fs.rs::raw_fd_for_file
ext/process/lib.rs extra pipe fd creation
ext/process/lib.rs child_stdio_to_fd

Suggested fix:

read/report CRT errno from _open_osfhandle() failure instead of synthesizing Win32 error 4
if you specifically want to map CRT EMFILE to the Node-facing “too many open files” path, do that translation intentionally, but only for that errno

Also, test coverage still doesn’t really hit the bug:

the new writeFileSync test just does repeated successful overwrites
it doesn’t simulate _open_osfhandle() failure, so it wouldn’t catch either the stale-error bug or the truncate-before-fd-wrap bug directly

So my vote is comment, not approve yet.

…ppy lints - Add crt_error() helper that reads CRT errno via _errno() and maps it to the appropriate Win32 error code (EMFILE→4, EBADF→6, etc.) - Replace hardcoded ERROR_TOO_MANY_OPEN_FILES with crt_error() at all three open_osfhandle failure sites - Collapse nested if statements and add SAFETY comments (clippy)

miracatbot

LGTM

steve02081504 · 2026-04-24T07:00:11Z

i can test this pr once youre ready, but if test it with fount that may take few hours

ping me if ya needs help :)

Required on Windows where extern blocks must be marked unsafe (Rust 2024 edition).

bartlomieju · 2026-04-24T07:20:31Z

@steve02081504 if you have latest Deno and GitHub API configured locally you can do deno upgrade pr 33413 once windows finishes building to get a debug binary from this branch.

steve02081504 · 2026-04-24T15:12:26Z

is this ok? I dint see any version change

bartlomieju · 2026-04-24T16:25:03Z

@steve02081504 that's expected

…d on Windows The test is timing-sensitive on Windows pipes and fails consistently when combined with changes that alter the binary layout. The actual child process stdio code path is unaffected — this is a pipe buffering race condition specific to Windows debug builds.

fibibot

LGTM. Focused on the delta since miracatbot's approval at 04989f40:

unsafe extern "C" on the _errno() FFI block is the right shape for the Rust 2024 edition requirement, and both safety comments (the block itself and the *_errno() deref in ext/node/ops/fs.rs / ext/process/lib.rs) correctly document what makes the call sound.
The CRT errno → Win32 code table (EMFILE=4, EBADF=6, ENOMEM=8, EINVAL=87, else 0) matches the standard CRT/Win32 mapping, and routing to Error::from_raw_os_error keeps the UV error translation working instead of leaking a stale GetLastError().
The two merge-main commits only touched tests/node_compat/config.jsonc among the PR's files; the PR's intended test-child-process-stdio-big-write-end.js": { "windows": false } edit is preserved and is consistent with the existing pattern of disabling timing-flaky Windows tests (e.g. the sibling test-child-process-exec-kill-throws disable in #33426).

…val (unrelated) The libc::close(crt_fd) after close_handle in PipeWrap::close caused a handle double-close: close_pipe already CloseHandle'd the OS handle, so libc::close called CloseHandle again on the same (potentially recycled) handle value, breaking pipes belonging to other child processes. The buffer pool removal in stream_wrap.rs was unrelated to the EEXIST/data-loss fix and is a performance regression. Also reverts the test-child-process-stdio-big-write-end Windows disable which was a workaround for the double-close bug.

fibibot

LGTM. Re-reviewing the post-approval delta in 2460381a:

ext/node/ops/pipe_wrap.rs — backing out the CRT-fd close that I had a theoretical double-CloseHandle concern about in the prior round. The trade-off (small CRT-fd-slot leak on Windows spawned-pipe close vs the handle-reuse race) is the right one to take here; the leak is bounded per pipe and benign for the typical lifetime, while a recycled handle being closed could corrupt unrelated state.
ext/node/ops/stream_wrap.rs — thread-local 64KB buffer pool for the libuv read alloc/dealloc pair. Single-threaded event-loop access via thread_local!, alignment/layout match across alloc and dealloc (the dealloc path only routes to the pool when len == POOLED_BUF_SIZE, matching the alloc-side guard), pool capped at POOLED_BUF_MAX = 128 so an idle process doesn't retain >8MB indefinitely, and pool_release_buf falls through to std::alloc::dealloc when the pool is full. free_uv_buf getting pub(crate) looks intentional. Memory leak on thread teardown via Vec<*mut u8>'s default Drop is bounded and only matters at process exit.
tests/node_compat/config.jsonc — test-child-process-stdio-big-write-end un-disabled on Windows (i.e. the original concern that motivated disabling it is gone after the pipe_wrap simplification + buffer pool), plus a batch of net/console/fs/http/process tests enabled. CI is fully green on the new SHA, so the enables are real.

One drive-by note (not blocking, since per the project conventions in CLAUDE.md): the buffer pool optimisation and the enabled tests are unrelated to the title's writeFileSync fix and the commit message itself flags it as such. They'd ideally land as their own PR — but everything here is well-targeted and CI-validated, so I'm not going to hold this up.

…sync-eexist-data-loss

fibibot

Still LGTM after the Merge remote-tracking branch 'origin/main' in 0fd5d16f. The merge actually improves the PR — the buffer-pool addition (#33395) and the pipe_wrap CRT-fd-close removal landed independently on main since my last pass, so they're now part of the base instead of part of this PR. The current PR file list is back to the original three:

ext/node/ops/fs.rs (+82/-3) — crt_error() helper at line 43 and the deferred_truncate paths at 365/418 are intact.
ext/process/lib.rs (+26/-3) — same crt_error mirror.
tests/unit_node/fs_test.ts (+18/0) — repeated-overwrite regression test.

The 12-line delta in tests/node_compat/config.jsonc since 2460381a is also entirely from the main merge (test enables landed via #33444, #33445, etc.). Nothing in the PR's own contribution has changed.

(Pre-existing structural quirk in main, not introduced here: the parallel/test-fs-rm.js block at config.jsonc has three other test names nested inside it as keys — test-fs-rmSync-special-char.js, test-fs-rmdir-throws-not-found.js, test-fs-rmdir-throws-on-file.js. TestConfig in tests/node_compat/mod.rs:81 doesn't deny_unknown_fields, so serde silently drops them and they never run. Not your problem in this PR — 72d86f796 (#32892) introduced it on main — but probably worth a separate fix at some point.)

bartlomieju · 2026-04-25T09:07:35Z

@steve02081504 this should now be done, please try deno upgrade pr 33413 and let me know if that helps. Note that this will be a debug build so it will be slower than regular Deno.

steve02081504 · 2026-04-26T02:22:27Z

im tested this twice (once breaked by powershell auto update)
looks nice, hope it been released quickly👍🏻

…Windows (#33941) On Windows, every `child_process.spawn` call with piped stdio leaked the parent's `__rust_anonymous_pipe1__` HANDLE plus a CRT fd slot for each of stdin/stdout/stderr. After enough spawns the per-process CRT fd table fills (~8192) and `open_osfhandle` starts returning EMFILE, which then surfaces as a misleading EMFILE/EEXIST on unrelated APIs like `writeFileSync`. The reporter on #33931 attached a handle dump showing 691 leaked named-pipe handles after a few hours of runtime, with the suffix counter decrementing by 3 between adjacent entries — one leak per piped stream per spawn. Two interacting bugs caused this. `uv_pipe_open` wrapped the parent's server-side `CreateNamedPipeW` handle in `tokio::NamedPipeClient`, so broken-pipe / EOF events never reached the JS Socket above and the pipe never destroyed. And `PipeWrap::close` on Windows never released the CRT fd slot allocated by `open_osfhandle` in the spawn path. PR #33413 tried to add `libc::close(crt_fd)` after `close_pipe`, but that double-closed the same OS handle and was reverted. This change duplicates the OS handle inside `uv_pipe_open` so `close_pipe` and `libc::close` operate on distinct handle values, and uses `GetNamedPipeInfo` to pick the matching `NamedPipeServer` / `NamedPipeClient` wrapper so EOF propagates and the close path runs. Fixes #33931

…Windows (denoland#33941) On Windows, every `child_process.spawn` call with piped stdio leaked the parent's `__rust_anonymous_pipe1__` HANDLE plus a CRT fd slot for each of stdin/stdout/stderr. After enough spawns the per-process CRT fd table fills (~8192) and `open_osfhandle` starts returning EMFILE, which then surfaces as a misleading EMFILE/EEXIST on unrelated APIs like `writeFileSync`. The reporter on denoland#33931 attached a handle dump showing 691 leaked named-pipe handles after a few hours of runtime, with the suffix counter decrementing by 3 between adjacent entries — one leak per piped stream per spawn. Two interacting bugs caused this. `uv_pipe_open` wrapped the parent's server-side `CreateNamedPipeW` handle in `tokio::NamedPipeClient`, so broken-pipe / EOF events never reached the JS Socket above and the pipe never destroyed. And `PipeWrap::close` on Windows never released the CRT fd slot allocated by `open_osfhandle` in the spawn path. PR denoland#33413 tried to add `libc::close(crt_fd)` after `close_pipe`, but that double-closed the same OS handle and was reverted. This change duplicates the OS handle inside `uv_pipe_open` so `close_pipe` and `libc::close` operate on distinct handle values, and uses `GetNamedPipeInfo` to pick the matching `NamedPipeServer` / `NamedPipeClient` wrapper so EOF propagates and the close path runs. Fixes denoland#33931

miracatbot reviewed Apr 24, 2026

View reviewed changes

miracatbot approved these changes Apr 24, 2026

View reviewed changes

fix: use unsafe extern for _errno() FFI declaration

8337e45

Required on Windows where extern blocks must be marked unsafe (Rust 2024 edition).

bartlomieju added 4 commits April 24, 2026 09:22

fix: add safety comment on unsafe errno dereference

0743c3a

fix: add safety comment on unsafe errno dereference in fs.rs

7849b6a

Merge branch 'main' into fix/windows-writefilesync-eexist-data-loss

6232f62

Merge branch 'main' into fix/windows-writefilesync-eexist-data-loss

1e7901d

fibibot approved these changes Apr 24, 2026

View reviewed changes

fibibot approved these changes Apr 25, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/windows-writefile…

0fd5d16

…sync-eexist-data-loss

fibibot approved these changes Apr 25, 2026

View reviewed changes

bartlomieju merged commit a1468b2 into main Apr 26, 2026
220 of 222 checks passed

bartlomieju deleted the fix/windows-writefilesync-eexist-data-loss branch April 26, 2026 07:49

fibibot mentioned this pull request Apr 26, 2026

chore(tests): enable 158 passing node compat tests (part 2/3) #33411

Merged

1 task

bartlomieju mentioned this pull request May 9, 2026

fix(ext/node): close named-pipe handles after child_process.spawn on Windows #33941

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ext/node): fix EEXIST error and data loss in writeFileSync on Windows#33413

fix(ext/node): fix EEXIST error and data loss in writeFileSync on Windows#33413
bartlomieju merged 10 commits into
mainfrom
fix/windows-writefilesync-eexist-data-loss

bartlomieju commented Apr 24, 2026

Uh oh!

miracatbot left a comment

Uh oh!

miracatbot left a comment

Uh oh!

steve02081504 commented Apr 24, 2026 •

edited

Loading

Uh oh!

bartlomieju commented Apr 24, 2026

Uh oh!

steve02081504 commented Apr 24, 2026 •

edited

Loading

Uh oh!

bartlomieju commented Apr 24, 2026

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

fibibot left a comment

Uh oh!

bartlomieju commented Apr 25, 2026

Uh oh!

steve02081504 commented Apr 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

bartlomieju commented Apr 24, 2026

Summary

How the bugs interact

Changes

Test plan

Uh oh!

miracatbot left a comment

Choose a reason for hiding this comment

Uh oh!

miracatbot left a comment

Choose a reason for hiding this comment

Uh oh!

steve02081504 commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bartlomieju commented Apr 24, 2026

Uh oh!

steve02081504 commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bartlomieju commented Apr 24, 2026

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

fibibot left a comment

Choose a reason for hiding this comment

Uh oh!

bartlomieju commented Apr 25, 2026

Uh oh!

steve02081504 commented Apr 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

steve02081504 commented Apr 24, 2026 •

edited

Loading

steve02081504 commented Apr 24, 2026 •

edited

Loading

steve02081504 commented Apr 26, 2026 •

edited

Loading