Skip to content

Commit

Permalink
fs: add support for AbortSignal in readFile
Browse files Browse the repository at this point in the history
PR-URL: #35911
Backport-PR-URL: #38386
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
  • Loading branch information
benjamingr authored and targos committed Apr 30, 2021
1 parent 88f4261 commit 5f88b64
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 14 deletions.
42 changes: 42 additions & 0 deletions doc/api/fs.md
Expand Up @@ -3026,6 +3026,10 @@ If `options.withFileTypes` is set to `true`, the result will contain
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35911
description: The options argument may include an AbortSignal to abort an
ongoing readFile request.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/12562
description: The `callback` parameter is no longer optional. Not passing
Expand All @@ -3051,6 +3055,7 @@ changes:
* `options` {Object|string}
* `encoding` {string|null} **Default:** `null`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
* `signal` {AbortSignal} allows aborting an in-progress readFile
* `callback` {Function}
* `err` {Error}
* `data` {string|Buffer}
Expand Down Expand Up @@ -3092,9 +3097,25 @@ fs.readFile('<directory>', (err, data) => {
});
```

It is possible to abort an ongoing request using an `AbortSignal`. If a
request is aborted the callback is called with an `AbortError`:

```js
const controller = new AbortController();
const signal = controller.signal;
fs.readFile(fileInfo[0].name, { signal }, (err, buf) => {
// ...
});
// When you want to abort the request
controller.abort();
```

The `fs.readFile()` function buffers the entire file. To minimize memory costs,
when possible prefer streaming via `fs.createReadStream()`.

Aborting an ongoing request does not abort individual operating
system requests but rather the internal buffering `fs.readFile` performs.

### File descriptors

1. Any specified file descriptor has to support reading.
Expand Down Expand Up @@ -4748,6 +4769,7 @@ added: v10.0.0

* `options` {Object|string}
* `encoding` {string|null} **Default:** `null`
* `signal` {AbortSignal} allows aborting an in-progress readFile
* Returns: {Promise}

Asynchronously reads the entire contents of a file.
Expand Down Expand Up @@ -5411,12 +5433,18 @@ print('./').catch(console.error);
### `fsPromises.readFile(path[, options])`
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/35911
description: The options argument may include an AbortSignal to abort an
ongoing readFile request.
-->

* `path` {string|Buffer|URL|FileHandle} filename or `FileHandle`
* `options` {Object|string}
* `encoding` {string|null} **Default:** `null`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
* `signal` {AbortSignal} allows aborting an in-progress readFile
* Returns: {Promise}

Asynchronously reads the entire contents of a file.
Expand All @@ -5432,6 +5460,20 @@ platform-specific. On macOS, Linux, and Windows, the promise will be rejected
with an error. On FreeBSD, a representation of the directory's contents will be
returned.

It is possible to abort an ongoing `readFile` using an `AbortSignal`. If a
request is aborted the promise returned is rejected with an `AbortError`:

```js
const controller = new AbortController();
const signal = controller.signal;
readFile(fileName, { signal }).then((file) => { /* ... */ });
// Abort the request
controller.abort();
```

Aborting an ongoing request does not abort individual operating
system requests but rather the internal buffering `fs.readFile` performs.

Any specified `FileHandle` has to support reading.

### `fsPromises.readlink(path[, options])`
Expand Down
3 changes: 3 additions & 0 deletions lib/fs.js
Expand Up @@ -316,6 +316,9 @@ function readFile(path, options, callback) {
const context = new ReadFileContext(callback, options.encoding);
context.isUserFd = isFd(path); // File descriptor ownership

if (options.signal) {
context.signal = options.signal;
}
if (context.isUserFd) {
process.nextTick(function tick(context) {
readFileAfterOpen.call({ context }, null, path);
Expand Down
25 changes: 23 additions & 2 deletions lib/internal/fs/promises.js
Expand Up @@ -30,12 +30,14 @@ const {
} = internalBinding('constants').fs;
const binding = internalBinding('fs');
const { Buffer } = require('buffer');

const { codes, hideStackFrames } = require('internal/errors');
const {
ERR_FS_FILE_TOO_LARGE,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_METHOD_NOT_IMPLEMENTED
} = require('internal/errors').codes;
ERR_METHOD_NOT_IMPLEMENTED,
} = codes;
const { isArrayBufferView } = require('internal/util/types');
const { rimrafPromises } = require('internal/fs/rimraf');
const {
Expand Down Expand Up @@ -83,6 +85,13 @@ const {
const getDirectoryEntriesPromise = promisify(getDirents);
const validateRmOptionsPromise = promisify(validateRmOptions);

let DOMException;
const lazyDOMException = hideStackFrames((message, name) => {
if (DOMException === undefined)
DOMException = internalBinding('messaging').DOMException;
return new DOMException(message, name);
});

class FileHandle extends JSTransferable {
constructor(filehandle) {
super();
Expand Down Expand Up @@ -260,8 +269,17 @@ async function writeFileHandle(filehandle, data) {
}

async function readFileHandle(filehandle, options) {
const signal = options && options.signal;

if (signal && signal.aborted) {
throw lazyDOMException('The operation was aborted', 'AbortError');
}
const statFields = await binding.fstat(filehandle.fd, false, kUsePromises);

if (signal && signal.aborted) {
throw lazyDOMException('The operation was aborted', 'AbortError');
}

let size;
if ((statFields[1/* mode */] & S_IFMT) === S_IFREG) {
size = statFields[8/* size */];
Expand All @@ -278,6 +296,9 @@ async function readFileHandle(filehandle, options) {
MathMin(size, kReadFileMaxChunkSize);
let endOfFile = false;
do {
if (signal && signal.aborted) {
throw lazyDOMException('The operation was aborted', 'AbortError');
}
const buf = Buffer.alloc(chunkSize);
const { bytesRead, buffer } =
await read(filehandle, buf, 0, chunkSize, -1);
Expand Down
16 changes: 16 additions & 0 deletions lib/internal/fs/read_file_context.js
Expand Up @@ -8,6 +8,16 @@ const { Buffer } = require('buffer');

const { FSReqCallback, close, read } = internalBinding('fs');

const { hideStackFrames } = require('internal/errors');


let DOMException;
const lazyDOMException = hideStackFrames((message, name) => {
if (DOMException === undefined)
DOMException = internalBinding('messaging').DOMException;
return new DOMException(message, name);
});

// Use 64kb in case the file type is not a regular file and thus do not know the
// actual file size. Increasing the value further results in more frequent over
// allocation for small files and consumes CPU time and memory that should be
Expand Down Expand Up @@ -74,13 +84,19 @@ class ReadFileContext {
this.pos = 0;
this.encoding = encoding;
this.err = null;
this.signal = undefined;
}

read() {
let buffer;
let offset;
let length;

if (this.signal && this.signal.aborted) {
return this.close(
lazyDOMException('The operation was aborted', 'AbortError')
);
}
if (this.size === 0) {
buffer = Buffer.allocUnsafeSlow(kReadFileUnknownBufferLength);
offset = 0;
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/fs/utils.js
Expand Up @@ -37,6 +37,7 @@ const {
const { once } = require('internal/util');
const { toPathIfFileURL } = require('internal/url');
const {
validateAbortSignal,
validateBoolean,
validateInt32,
validateUint32
Expand Down Expand Up @@ -297,6 +298,10 @@ function getOptions(options, defaultOptions) {

if (options.encoding !== 'buffer')
assertEncoding(options.encoding);

if (options.signal !== undefined) {
validateAbortSignal(options.signal, 'options.signal');
}
return options;
}

Expand Down
50 changes: 38 additions & 12 deletions test/parallel/test-fs-promises-readfile.js
@@ -1,3 +1,4 @@
// Flags: --experimental-abortcontroller
'use strict';

const common = require('../common');
Expand All @@ -10,18 +11,21 @@ tmpdir.refresh();

const fn = path.join(tmpdir.path, 'large-file');

async function validateReadFile() {
// Creating large buffer with random content
const buffer = Buffer.from(
Array.apply(null, { length: 16834 * 2 })
.map(Math.random)
.map((number) => (number * (1 << 8)))
);
// Creating large buffer with random content
const largeBuffer = Buffer.from(
Array.apply(null, { length: 16834 * 2 })
.map(Math.random)
.map((number) => (number * (1 << 8)))
);

async function createLargeFile() {
// Writing buffer to a file then try to read it
await writeFile(fn, buffer);
await writeFile(fn, largeBuffer);
}

async function validateReadFile() {
const readBuffer = await readFile(fn);
assert.strictEqual(readBuffer.equals(buffer), true);
assert.strictEqual(readBuffer.equals(largeBuffer), true);
}

async function validateReadFileProc() {
Expand All @@ -39,6 +43,28 @@ async function validateReadFileProc() {
assert.ok(hostname.length > 0);
}

validateReadFile()
.then(() => validateReadFileProc())
.then(common.mustCall());
function validateReadFileAbortLogicBefore() {
const controller = new AbortController();
const signal = controller.signal;
controller.abort();
assert.rejects(readFile(fn, { signal }), {
name: 'AbortError'
});
}

function validateReadFileAbortLogicDuring() {
const controller = new AbortController();
const signal = controller.signal;
process.nextTick(() => controller.abort());
assert.rejects(readFile(fn, { signal }), {
name: 'AbortError'
});
}

(async () => {
await createLargeFile();
await validateReadFile();
await validateReadFileProc();
await validateReadFileAbortLogicBefore();
await validateReadFileAbortLogicDuring();
})().then(common.mustCall());
19 changes: 19 additions & 0 deletions test/parallel/test-fs-readfile.js
@@ -1,3 +1,4 @@
// Flags: --experimental-abortcontroller
'use strict';
const common = require('../common');

Expand Down Expand Up @@ -57,3 +58,21 @@ for (const e of fileInfo) {
assert.deepStrictEqual(buf, e.contents);
}));
}
{
// Test cancellation, before
const controller = new AbortController();
const signal = controller.signal;
controller.abort();
fs.readFile(fileInfo[0].name, { signal }, common.mustCall((err, buf) => {
assert.strictEqual(err.name, 'AbortError');
}));
}
{
// Test cancellation, during read
const controller = new AbortController();
const signal = controller.signal;
fs.readFile(fileInfo[0].name, { signal }, common.mustCall((err, buf) => {
assert.strictEqual(err.name, 'AbortError');
}));
process.nextTick(() => controller.abort());
}

0 comments on commit 5f88b64

Please sign in to comment.