diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-05-07 03:16:17 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2020-05-15 20:36:50 +0300 |
commit | d3a8a23089af06bb047bf9bad7531fbfc70f6314 (patch) | |
tree | d59002054b4dcd34b1fe49cd369ed570f43df619 /lib/internal/fs | |
parent | e9f293750760d59243020d0376edf242c9a26b67 (diff) |
fs: forbid concurrent operations on Dir handle
libuv does not expect concurrent operations on `uv_dir_t` instances,
and will gladly create memory leaks, corrupt data, or crash the
process.
This patch forbids that, and:
- Makes sure that concurrent async operations are run sequentially
- Throws an exception if sync operations are attempted during an
async operation
The assumption here is that a thrown exception is preferable to
a potential hard crash.
This fully fixes flakiness from `parallel/test-fs-opendir` when
run under ASAN.
PR-URL: https://github.com/nodejs/node/pull/33274
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Diffstat (limited to 'lib/internal/fs')
-rw-r--r-- | lib/internal/fs/dir.js | 35 |
1 files changed, 35 insertions, 0 deletions
diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index e23b2f7725b..b4cd72d3668 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -12,6 +12,7 @@ const dirBinding = internalBinding('fs_dir'); const { codes: { ERR_DIR_CLOSED, + ERR_DIR_CONCURRENT_OPERATION, ERR_INVALID_CALLBACK, ERR_MISSING_ARGS } @@ -37,6 +38,7 @@ const kDirOptions = Symbol('kDirOptions'); const kDirReadImpl = Symbol('kDirReadImpl'); const kDirReadPromisified = Symbol('kDirReadPromisified'); const kDirClosePromisified = Symbol('kDirClosePromisified'); +const kDirOperationQueue = Symbol('kDirOperationQueue'); class Dir { constructor(handle, path, options) { @@ -46,6 +48,10 @@ class Dir { this[kDirPath] = path; this[kDirClosed] = false; + // Either `null` or an Array of pending operations (= functions to be called + // once the current operation is done). + this[kDirOperationQueue] = null; + this[kDirOptions] = { bufferSize: 32, ...getOptions(options, { @@ -79,6 +85,13 @@ class Dir { throw new ERR_INVALID_CALLBACK(callback); } + if (this[kDirOperationQueue] !== null) { + this[kDirOperationQueue].push(() => { + this[kDirReadImpl](maybeSync, callback); + }); + return; + } + if (this[kDirBufferedEntries].length > 0) { const [ name, type ] = this[kDirBufferedEntries].splice(0, 2); if (maybeSync) @@ -90,6 +103,12 @@ class Dir { const req = new FSReqCallback(); req.oncomplete = (err, result) => { + process.nextTick(() => { + const queue = this[kDirOperationQueue]; + this[kDirOperationQueue] = null; + for (const op of queue) op(); + }); + if (err || result === null) { return callback(err, result); } @@ -98,6 +117,7 @@ class Dir { getDirent(this[kDirPath], result[0], result[1], callback); }; + this[kDirOperationQueue] = []; this[kDirHandle].read( this[kDirOptions].encoding, this[kDirOptions].bufferSize, @@ -110,6 +130,10 @@ class Dir { throw new ERR_DIR_CLOSED(); } + if (this[kDirOperationQueue] !== null) { + throw new ERR_DIR_CONCURRENT_OPERATION(); + } + if (this[kDirBufferedEntries].length > 0) { const [ name, type ] = this[kDirBufferedEntries].splice(0, 2); return getDirent(this[kDirPath], name, type); @@ -143,6 +167,13 @@ class Dir { throw new ERR_INVALID_CALLBACK(callback); } + if (this[kDirOperationQueue] !== null) { + this[kDirOperationQueue].push(() => { + this.close(callback); + }); + return; + } + this[kDirClosed] = true; const req = new FSReqCallback(); req.oncomplete = callback; @@ -154,6 +185,10 @@ class Dir { throw new ERR_DIR_CLOSED(); } + if (this[kDirOperationQueue] !== null) { + throw new ERR_DIR_CONCURRENT_OPERATION(); + } + this[kDirClosed] = true; const ctx = { path: this[kDirPath] }; const result = this[kDirHandle].close(undefined, ctx); |