From 743b6122e53137c0966b1ac0036493a10b3b0d5c Mon Sep 17 00:00:00 2001 From: mohin-io Date: Sat, 18 Oct 2025 05:40:16 +0100 Subject: [PATCH] fs: improve recursive opendir buffer handling --- lib/internal/fs/dir.js | 117 +++++++++++++++---- test/sequential/test-fs-opendir-recursive.js | 13 +++ 2 files changed, 106 insertions(+), 24 deletions(-) diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 03f585bab2afaf..33cf52e5f1a11c 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -79,7 +79,7 @@ class Dir { return this.#path; } - #processHandlerQueue() { + #processHandlerQueueSync() { while (this.#handlerQueue.length > 0) { const handler = ArrayPrototypeShift(this.#handlerQueue); const { handle, path } = handler; @@ -106,6 +106,89 @@ class Dir { return this.#bufferedEntries.length > 0; } + #processHandlerQueueAsync(maybeSync, callback) { + while (this.#handlerQueue.length > 0) { + const handler = ArrayPrototypeShift(this.#handlerQueue); + const { handle, path } = handler; + + if (handler.pending === true) { + ArrayPrototypePush(this.#handlerQueue, handler); + break; + } + + handler.pending = true; + this.#operationQueue = []; + + const req = new FSReqCallback(); + req.oncomplete = (err, result) => { + process.nextTick(() => { + const queue = this.#operationQueue; + this.#operationQueue = null; + for (const op of queue) op(); + }); + + handler.pending = false; + + if (err) { + handle.close(); + callback(err); + return; + } + + if (result === null) { + handle.close(); + this.#readImpl(maybeSync, callback); + return; + } + + try { + this.#processReadResult(path, result); + if (result.length > 0) { + ArrayPrototypePush(this.#handlerQueue, handler); + } else { + handle.close(); + } + this.#emitBufferedDirent(maybeSync, callback); + } catch (error) { + callback(error); + } + }; + + handle.read( + this.#options.encoding, + this.#options.bufferSize, + req, + ); + return true; + } + + return false; + } + + #emitBufferedDirent(maybeSync, callback) { + if (this.#bufferedEntries.length === 0) { + return false; + } + + try { + const dirent = ArrayPrototypeShift(this.#bufferedEntries); + + if (this.#options.recursive && dirent.isDirectory()) { + this.#readSyncRecursive(dirent); + } + + if (maybeSync) { + process.nextTick(callback, null, dirent); + } else { + callback(null, dirent); + } + } catch (error) { + callback(error); + } + + return true; + } + read(callback) { return arguments.length === 0 ? this.#readPromisified() : this.#readImpl(true, callback); } @@ -121,6 +204,10 @@ class Dir { validateFunction(callback, 'callback'); + if (this.#emitBufferedDirent(maybeSync, callback)) { + return; + } + if (this.#operationQueue !== null) { ArrayPrototypePush(this.#operationQueue, () => { this.#readImpl(maybeSync, callback); @@ -128,22 +215,8 @@ class Dir { return; } - if (this.#processHandlerQueue()) { - try { - const dirent = ArrayPrototypeShift(this.#bufferedEntries); - - if (this.#options.recursive && dirent.isDirectory()) { - this.#readSyncRecursive(dirent); - } - - if (maybeSync) - process.nextTick(callback, null, dirent); - else - callback(null, dirent); - return; - } catch (error) { - return callback(error); - } + if (this.#processHandlerQueueAsync(maybeSync, callback)) { + return; } const req = new FSReqCallback(); @@ -160,11 +233,7 @@ class Dir { try { this.#processReadResult(this.#path, result); - const dirent = ArrayPrototypeShift(this.#bufferedEntries); - if (this.#options.recursive && dirent.isDirectory()) { - this.#readSyncRecursive(dirent); - } - callback(null, dirent); + this.#emitBufferedDirent(maybeSync, callback); } catch (error) { callback(error); } @@ -202,7 +271,7 @@ class Dir { return; } - ArrayPrototypePush(this.#handlerQueue, { handle, path }); + ArrayPrototypePush(this.#handlerQueue, { handle, path, pending: false }); } readSync() { @@ -214,7 +283,7 @@ class Dir { throw new ERR_DIR_CONCURRENT_OPERATION(); } - if (this.#processHandlerQueue()) { + if (this.#processHandlerQueueSync()) { const dirent = ArrayPrototypeShift(this.#bufferedEntries); if (this.#options.recursive && dirent.isDirectory()) { this.#readSyncRecursive(dirent); diff --git a/test/sequential/test-fs-opendir-recursive.js b/test/sequential/test-fs-opendir-recursive.js index 4ad3230c01b3cb..4013815d07c8b4 100644 --- a/test/sequential/test-fs-opendir-recursive.js +++ b/test/sequential/test-fs-opendir-recursive.js @@ -223,6 +223,19 @@ function processDirCb(dir, cb) { test().then(common.mustCall()); } +{ + async function testBufferSize() { + const dir = await fsPromises.opendir(testDir, { recursive: true, bufferSize: 1 }); + const dirents = []; + for await (const dirent of dir) { + dirents.push(dirent); + } + assertDirents(dirents); + } + + testBufferSize().then(common.mustCall()); +} + // Issue https://github.com/nodejs/node/issues/48820 highlights that // opendir recursive does not properly handle the buffer size option. // This test asserts that the buffer size option is respected.