Repository: nodejs/node
Pull request: lib: resolve the issue of not adhering to the specified buffersize

fixes #55764

We create a `queueHandler`, and in every iteration we execute the handlers in the `queueHandler` until we get a non-null result.

Changed files:
filename: lib/internal/fs/dir.js
patch: @@ -40,9 +40,8 @@ class Dir {
   #options;
   #readPromisified;
   #closePromisified;
-  // Either `null` or an Array of pending operations (= functions to be called
-  // once the current operation is done).
   #operationQueue = null;
+  #handlerQueue = [];
 
   constructor(handle, path, options) {
     if (handle == null) throw new ERR_MISSING_ARGS('handle');
@@ -67,6 +66,33 @@ class Dir {
     return this.#path;
   }
 
+  #processHandlerQueue() {
+    while (this.#handlerQueue.length > 0) {
+      const handler = ArrayPrototypeShift(this.#handlerQueue);
+      const { handle, path } = handler;
+
+      const result = handle.read(
+        this.#options.encoding,
+        this.#options.bufferSize,
+      );
+
+      if (result !== null) {
+        this.processReadResult(path, result);
+        if (result.length > 0) {
+          ArrayPrototypePush(this.#handlerQueue, handler);
+        }
+      } else {
+        handle.close();
+      }
+
+      if (this.#bufferedEntries.length > 0) {
+        break;
+      }
+    }
+
+    return this.#bufferedEntries.length > 0;
+  }
+
   read(callback) {
     return this.#readImpl(true, callback);
   }
@@ -89,7 +115,7 @@ class Dir {
       return;
     }
 
-    if (this.#bufferedEntries.length > 0) {
+    if (this.#processHandlerQueue()) {
       try {
         const dirent = ArrayPrototypeShift(this.#bufferedEntries);
 
@@ -159,25 +185,11 @@ class Dir {
       this.#options.encoding,
     );
 
-    // Terminate early, since it's already thrown.
     if (handle === undefined) {
       return;
     }
 
-    // Fully read the directory and buffer the entries.
-    // This is a naive solution and for very large sub-directories
-    // it can even block the event loop. Essentially, `bufferSize` is
-    // not respected for recursive mode. This is a known limitation.
-    // Issue to fix: https://github.com/nodejs/node/issues/55764
-    let result;
-    while ((result = handle.read(
-      this.#options.encoding,
-      this.#options.bufferSize,
-    ))) {
-      this.processReadResult(path, result);
-    }
-
-    handle.close();
+    ArrayPrototypePush(this.#handlerQueue, { handle, path });
   }
 
   readSync() {
@@ -189,7 +201,7 @@ class Dir {
       throw new ERR_DIR_CONCURRENT_OPERATION();
     }
 
-    if (this.#bufferedEntries.length > 0) {
+    if (this.#processHandlerQueue()) {
       const dirent = ArrayPrototypeShift(this.#bufferedEntries);
       if (this.#options.recursive && dirent.isDirectory()) {
         this.readSyncRecursive(dirent);
@@ -216,15 +228,13 @@ class Dir {
   }
 
   close(callback) {
-    // Promise
     if (callback === undefined) {
       if (this.#closed === true) {
         return PromiseReject(new ERR_DIR_CLOSED());
       }
       return this.#closePromisified();
     }
 
-    // callback
     validateFunction(callback, 'callback');
 
     if (this.#closed === true) {
@@ -239,6 +249,11 @@ class Dir {
       return;
     }
 
+    while (this.#handlerQueue.length > 0) {
+      const handler = ArrayPrototypeShift(this.#handlerQueue);
+      handler.handle.close();
+    }
+
     this.#closed = true;
     const req = new FSReqCallback();
     req.oncomplete = callback;
@@ -254,6 +269,11 @@ class Dir {
       throw new ERR_DIR_CONCURRENT_OPERATION();
     }
 
+    while (this.#handlerQueue.length > 0) {
+      const handler = ArrayPrototypeShift(this.#handlerQueue);
+      handler.handle.close();
+    }
+
     this.#closed = true;
     this.#handle.close();
   }