From dba2d275546173d0b4380befc97827aa655706c3 Mon Sep 17 00:00:00 2001 From: cu8code Date: Sun, 17 Nov 2024 22:00:03 +0530 Subject: [PATCH] lib: resolve the issue of not adhering to the specified buffer size We create a `queueHandler`, and in every iteration we execute the handlers in the `queueHandler` until we get a non-null result. --- lib/internal/fs/dir.js | 62 ++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 0d7f675643dfcd..f227b51af03669 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -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,7 +228,6 @@ class Dir { } close(callback) { - // Promise if (callback === undefined) { if (this.#closed === true) { return PromiseReject(new ERR_DIR_CLOSED()); @@ -224,7 +235,6 @@ class Dir { 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(); }