Skip to content

Commit a8a86b3

Browse files
authored
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. PR-URL: #55896 Refs: #55764 Reviewed-By: Ethan Arrowood <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent cf896c3 commit a8a86b3

File tree

1 file changed

+41
-21
lines changed

1 file changed

+41
-21
lines changed

lib/internal/fs/dir.js

+41-21
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@ class Dir {
4040
#options;
4141
#readPromisified;
4242
#closePromisified;
43-
// Either `null` or an Array of pending operations (= functions to be called
44-
// once the current operation is done).
4543
#operationQueue = null;
44+
#handlerQueue = [];
4645

4746
constructor(handle, path, options) {
4847
if (handle == null) throw new ERR_MISSING_ARGS('handle');
@@ -67,6 +66,33 @@ class Dir {
6766
return this.#path;
6867
}
6968

69+
#processHandlerQueue() {
70+
while (this.#handlerQueue.length > 0) {
71+
const handler = ArrayPrototypeShift(this.#handlerQueue);
72+
const { handle, path } = handler;
73+
74+
const result = handle.read(
75+
this.#options.encoding,
76+
this.#options.bufferSize,
77+
);
78+
79+
if (result !== null) {
80+
this.processReadResult(path, result);
81+
if (result.length > 0) {
82+
ArrayPrototypePush(this.#handlerQueue, handler);
83+
}
84+
} else {
85+
handle.close();
86+
}
87+
88+
if (this.#bufferedEntries.length > 0) {
89+
break;
90+
}
91+
}
92+
93+
return this.#bufferedEntries.length > 0;
94+
}
95+
7096
read(callback) {
7197
return this.#readImpl(true, callback);
7298
}
@@ -89,7 +115,7 @@ class Dir {
89115
return;
90116
}
91117

92-
if (this.#bufferedEntries.length > 0) {
118+
if (this.#processHandlerQueue()) {
93119
try {
94120
const dirent = ArrayPrototypeShift(this.#bufferedEntries);
95121

@@ -159,25 +185,11 @@ class Dir {
159185
this.#options.encoding,
160186
);
161187

162-
// Terminate early, since it's already thrown.
163188
if (handle === undefined) {
164189
return;
165190
}
166191

167-
// Fully read the directory and buffer the entries.
168-
// This is a naive solution and for very large sub-directories
169-
// it can even block the event loop. Essentially, `bufferSize` is
170-
// not respected for recursive mode. This is a known limitation.
171-
// Issue to fix: https://github.com/nodejs/node/issues/55764
172-
let result;
173-
while ((result = handle.read(
174-
this.#options.encoding,
175-
this.#options.bufferSize,
176-
))) {
177-
this.processReadResult(path, result);
178-
}
179-
180-
handle.close();
192+
ArrayPrototypePush(this.#handlerQueue, { handle, path });
181193
}
182194

183195
readSync() {
@@ -189,7 +201,7 @@ class Dir {
189201
throw new ERR_DIR_CONCURRENT_OPERATION();
190202
}
191203

192-
if (this.#bufferedEntries.length > 0) {
204+
if (this.#processHandlerQueue()) {
193205
const dirent = ArrayPrototypeShift(this.#bufferedEntries);
194206
if (this.#options.recursive && dirent.isDirectory()) {
195207
this.readSyncRecursive(dirent);
@@ -216,15 +228,13 @@ class Dir {
216228
}
217229

218230
close(callback) {
219-
// Promise
220231
if (callback === undefined) {
221232
if (this.#closed === true) {
222233
return PromiseReject(new ERR_DIR_CLOSED());
223234
}
224235
return this.#closePromisified();
225236
}
226237

227-
// callback
228238
validateFunction(callback, 'callback');
229239

230240
if (this.#closed === true) {
@@ -239,6 +249,11 @@ class Dir {
239249
return;
240250
}
241251

252+
while (this.#handlerQueue.length > 0) {
253+
const handler = ArrayPrototypeShift(this.#handlerQueue);
254+
handler.handle.close();
255+
}
256+
242257
this.#closed = true;
243258
const req = new FSReqCallback();
244259
req.oncomplete = callback;
@@ -254,6 +269,11 @@ class Dir {
254269
throw new ERR_DIR_CONCURRENT_OPERATION();
255270
}
256271

272+
while (this.#handlerQueue.length > 0) {
273+
const handler = ArrayPrototypeShift(this.#handlerQueue);
274+
handler.handle.close();
275+
}
276+
257277
this.#closed = true;
258278
this.#handle.close();
259279
}

0 commit comments

Comments
 (0)