GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||
GeezerVison is showing github.com — fs: add windowsHandle option to file streams by PickBas · Pull Request #63851 · nodejs/node Conversation
|
|
Review requested:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@addaleax Could you please take another look at this PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Fixes: nodejs#57288 Signed-off-by: PickBas <[email protected]>
32c7076 to
a40b758
Compare
Signed-off-by: PickBas <[email protected]>
a40b758 to
36f2bcd
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## main #63851 +/- ## ========================================== + Coverage 90.29% 90.31% +0.02% ========================================== Files 759 759 Lines 247754 247788 +34 Branches 46721 46741 +20 ========================================== + Hits 223708 223798 +90 + Misses 15521 15468 -53 + Partials 8525 8522 -3
🚀 New features to boost your workflow:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Landed in d9d4446 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Fixes: #57288 Signed-off-by: PickBas <[email protected]> PR-URL: #63851 Reviewed-By: Stefan Stojanovic <[email protected]>
Fixes: #57288 Signed-off-by: PickBas <[email protected]> PR-URL: #63851 Reviewed-By: Stefan Stojanovic <[email protected]>
Fixes: #57288 Signed-off-by: PickBas <[email protected]> PR-URL: #63851 Reviewed-By: Stefan Stojanovic <[email protected]>
### What does this PR do? Adds the `windowsHandle` option to `fs.createReadStream` and `fs.createWriteStream`, so a raw Win32 `HANDLE` can back a file stream, as in Node 24.21 and 26.8 (nodejs/node#63851). Bun ignored the option. With an empty path, the stream resolved the current directory and failed with `EISDIR`. - `streams.ts` validates the option in Node's check order. - A small native binding wraps the handle with `uv_open_osfhandle`. The stream then owns the fd and closes it. - `bun-types` declares the option, because `@types/node` does not have it yet. It also fixes two related problems with fd 0: - Destroying a file stream skipped the close when its fd was 0. It now checks `fd == null`, as Node does since nodejs/node#66131, and Node's updated `test-fs-stream-fs-options.js` is ported. A `windowsHandle` can be wrapped as fd 0 if the process has closed its stdin. - Bun destroys `process.stdin` after `'end'`. With the change above, that would have closed the process's real stdin, so `process.stdin` now keeps fd 0 open when it is destroyed. A test in `process-stdin.test.ts` covers this. One deliberate difference: the handle is wrapped after `start`/`end` validation, so a failure there leaves the caller owning it. Node wraps first and leaks the fd in that case. An invalid `highWaterMark` or `signal` still leaks it, as in Node. No docs change: Bun's docs do not describe `node:fs` stream options. ### How did you verify your code works? - Buildkite build #133777 passed on every platform. Both new test files ran on Windows x64 (Server 2019) and Windows 11 arm64: `test/js/node/fs/fs-stream-windows-handle.test.ts` and the ported `test/js/node/test/parallel/test-fs-stream-windows-handle.js`. - On an unpatched Bun, the ported Node test fails at its first assertion and the `EISDIR` reproduces. - On an unpatched Bun, the ported fd 0 test fails, as expected. - The type fixture in `test/integration/bun-types/fixture/fs.ts` accepts a `bigint` and rejects a number. Not yet measured: a pending pipe read holds a libuv threadpool worker, which may matter with several streams open at once. Node has the same design. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Dylan Conway <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Windows anonymous pipes do not work with createReadStream
There was an error while loading. Please reload this page.