Skip to content

Commit

Permalink
lib: resolve the issue of not adhering to the specified buffer size
Browse files Browse the repository at this point in the history
We create a `queueHandler`, and in every iteration we execute
the handlers in the `queueHandler` until we get a non-null result.
  • Loading branch information
cu8code committed Dec 5, 2024
1 parent 839ad8b commit dba2d27
Showing 1 changed file with 41 additions and 21 deletions.
62 changes: 41 additions & 21 deletions lib/internal/fs/dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
}
Expand All @@ -89,7 +115,7 @@ class Dir {
return;
}

if (this.#bufferedEntries.length > 0) {
if (this.#processHandlerQueue()) {
try {
const dirent = ArrayPrototypeShift(this.#bufferedEntries);

Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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();
}
Expand Down

0 comments on commit dba2d27

Please sign in to comment.