FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: add autoClose option to FileHandle readableWebStream · nodejs/node@6f4c9dd · GitHub

/ node Public

Commit 6f4c9dd

Browse files
jasnell
authored and
targos
committed
fs: add autoClose option to FileHandle readableWebStream
By default, the `readableWebStream` method of `FileHandle` returns a ReadableStream that, when finished, does not close the underlying FileHandle. This can lead to issues if the stream is consumed without having a reference to the FileHandle to close after use. This commit adds an `autoClose` option to the `readableWebStream` method, which, when set to `true`, will automatically close the FileHandle when the stream is finished or canceled. The test modified in this commit demonstrates one of the cases where this is necessary in that the stream is consumed by separate code than the FileHandle which was being left to close the underlying fd when it is garbage collected, which is a deprecated behavior. PR-URL: #58548 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 88e621e commit 6f4c9dd

4 files changed

Lines changed: 58 additions & 13 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,11 +476,14 @@ Reads data from the file and stores that in the given buffer.
476476
If the file is not modified concurrently, the end-of-file is reached when the
477477
number of bytes read is zero.
478478
479-
#### `filehandle.readableWebStream()`
479+
#### `filehandle.readableWebStream([options])`
480480
481481
<!-- YAML
482482
added: v17.0.0
483483
changes:
484+
- version: REPLACEME
485+
pr-url: https://github.com/nodejs/node/pull/58548
486+
description: Added the `autoClose` option.
484487
- version: v24.0.0
485488
pr-url: https://github.com/nodejs/node/pull/57513
486489
description: Marking the API stable.
@@ -496,6 +499,9 @@ changes:
496499
description: Added option to create a 'bytes' stream.
497500
-->
498501
502+
* `options` {Object}
503+
* `autoClose` {boolean} When true, causes the {FileHandle} to be closed when the
504+
stream is closed. **Default:** `false`
499505
* Returns: {ReadableStream}
500506
501507
Returns a byte-oriented `ReadableStream` that may be used to read the file's

‎lib/internal/fs/promises.js‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ const {
8686
validateInteger,
8787
validateObject,
8888
validateOneOf,
89-
validateString,
9089
kValidateObjectAllowNullable,
9190
} = require('internal/validators');
9291
const pathModule = require('path');
@@ -279,9 +278,10 @@ class FileHandle extends EventEmitter {
279278
/**
280279
* @typedef {import('../webstreams/readablestream').ReadableStream
281280
* } ReadableStream
281+
* @param {{ type?: 'bytes', autoClose?: boolean }} [options]
282282
* @returns {ReadableStream}
283283
*/
284-
readableWebStream(options = { __proto__: null, type: 'bytes' }) {
284+
readableWebStream(options = kEmptyObject) {
285285
if (this[kFd] === -1)
286286
throw new ERR_INVALID_STATE('The FileHandle is closed');
287287
if (this[kClosePromise])
@@ -290,20 +290,27 @@ class FileHandle extends EventEmitter {
290290
throw new ERR_INVALID_STATE('The FileHandle is locked');
291291
this[kLocked] = true;
292292

293-
if (options.type !== undefined) {
294-
validateString(options.type, 'options.type');
295-
}
296-
if (options.type !== 'bytes') {
293+
validateObject(options, 'options');
294+
const {
295+
type = 'bytes',
296+
autoClose = false,
297+
} = options;
298+
299+
validateBoolean(autoClose, 'options.autoClose');
300+
301+
if (type !== 'bytes') {
297302
process.emitWarning(
298303
'A non-"bytes" options.type has no effect. A byte-oriented steam is ' +
299304
'always created.',
300305
'ExperimentalWarning',
301306
);
302307
}
303308

304-
305309
const readFn = FunctionPrototypeBind(this.read, this);
306-
const ondone = FunctionPrototypeBind(this[kUnref], this);
310+
const ondone = async () => {
311+
this[kUnref]();
312+
if (autoClose) await this.close();
313+
};
307314

308315
const ReadableStream = lazyReadableStream();
309316
const readable = new ReadableStream({
@@ -315,15 +322,15 @@ class FileHandle extends EventEmitter {
315322
const { bytesRead } = await readFn(view, view.byteOffset, view.byteLength);
316323

317324
if (bytesRead === 0) {
318-
ondone();
319325
controller.close();
326+
await ondone();
320327
}
321328

322329
controller.byobRequest.respond(bytesRead);
323330
},
324331

325-
cancel() {
326-
ondone();
332+
async cancel() {
333+
await ondone();
327334
},
328335
});
329336

‎test/es-module/test-wasm-web-api.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ function testCompileStreamingRejectionUsingFetch(responseCallback, rejection) {
106106
// Response whose body is a ReadableStream instead of calling fetch().
107107
await testCompileStreamingSuccess(async () => {
108108
const handle = await fs.open(fixtures.path('simple.wasm'));
109-
const stream = handle.readableWebStream();
109+
// We set the autoClose option to true so that the file handle is closed
110+
// automatically when the stream is completed or canceled.
111+
const stream = handle.readableWebStream({ autoClose: true });
110112
return Promise.resolve(new Response(stream, {
111113
status: 200,
112114
headers: { 'Content-Type': 'application/wasm' }

‎test/parallel/test-filehandle-autoclose.mjs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import '../common/index.mjs';
2+
import { open } from 'node:fs/promises';
3+
import { rejects } from 'node:assert';
4+
5+
{
6+
const fh = await open(new URL(import.meta.url));
7+
8+
// TODO: remove autoClose option when it becomes default
9+
const readableStream = fh.readableWebStream({ autoClose: true });
10+
11+
// Consume the stream
12+
await new Response(readableStream).text();
13+
14+
// If reading the FileHandle after the stream is consumed fails,
15+
// then we assume the autoClose option worked as expected.
16+
await rejects(fh.read(), { code: 'EBADF' });
17+
}
18+
19+
{
20+
await using fh = await open(new URL(import.meta.url));
21+
22+
const readableStream = fh.readableWebStream({ autoClose: false });
23+
24+
// Consume the stream
25+
await new Response(readableStream).text();
26+
27+
// Filehandle must be still open
28+
await fh.read();
29+
await fh.close();
30+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL