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>

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);