From 3aa0bc883c1b5036542f15aebe3b5c5ef013201b Mon Sep 17 00:00:00 2001 From: pluris Date: Mon, 4 Dec 2023 09:10:32 +0900 Subject: [PATCH 1/2] fs: use private fields instead of symbols for `Dir` --- lib/internal/fs/dir.js | 140 ++++++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 71 deletions(-) diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 82c6c1bd780fba..761e0f546909af 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -6,7 +6,6 @@ const { FunctionPrototypeBind, ObjectDefineProperty, PromiseReject, - Symbol, SymbolAsyncIterator, } = primordials; @@ -34,74 +33,73 @@ const { validateUint32, } = require('internal/validators'); -const kDirHandle = Symbol('kDirHandle'); -const kDirPath = Symbol('kDirPath'); -const kDirBufferedEntries = Symbol('kDirBufferedEntries'); -const kDirClosed = Symbol('kDirClosed'); -const kDirOptions = Symbol('kDirOptions'); -const kDirReadImpl = Symbol('kDirReadImpl'); -const kDirReadPromisified = Symbol('kDirReadPromisified'); -const kDirClosePromisified = Symbol('kDirClosePromisified'); -const kDirOperationQueue = Symbol('kDirOperationQueue'); - class Dir { + #dirHandle; + #dirPath; + #dirBufferedEntries; + #dirClosed; + #dirOptions; + #dirReadPromisified; + #dirClosePromisified; + #dirOperationQueue; + constructor(handle, path, options) { if (handle == null) throw new ERR_MISSING_ARGS('handle'); - this[kDirHandle] = handle; - this[kDirBufferedEntries] = []; - this[kDirPath] = path; - this[kDirClosed] = false; + this.#dirHandle = handle; + this.#dirBufferedEntries = []; + this.#dirPath = path; + this.#dirClosed = false; // Either `null` or an Array of pending operations (= functions to be called // once the current operation is done). - this[kDirOperationQueue] = null; + this.#dirOperationQueue = null; - this[kDirOptions] = { + this.#dirOptions = { bufferSize: 32, ...getOptions(options, { encoding: 'utf8', }), }; - validateUint32(this[kDirOptions].bufferSize, 'options.bufferSize', true); + validateUint32(this.#dirOptions.bufferSize, 'options.bufferSize', true); - this[kDirReadPromisified] = FunctionPrototypeBind( - internalUtil.promisify(this[kDirReadImpl]), this, false); - this[kDirClosePromisified] = FunctionPrototypeBind( + this.#dirReadPromisified = FunctionPrototypeBind( + internalUtil.promisify(this.#dirReadImpl), this, false); + this.#dirClosePromisified = FunctionPrototypeBind( internalUtil.promisify(this.close), this); } get path() { - return this[kDirPath]; + return this.#dirPath; } read(callback) { - return this[kDirReadImpl](true, callback); + return this.#dirReadImpl(true, callback); } - [kDirReadImpl](maybeSync, callback) { - if (this[kDirClosed] === true) { + #dirReadImpl(maybeSync, callback) { + if (this.#dirClosed === true) { throw new ERR_DIR_CLOSED(); } if (callback === undefined) { - return this[kDirReadPromisified](); + return this.#dirReadPromisified(); } validateFunction(callback, 'callback'); - if (this[kDirOperationQueue] !== null) { - ArrayPrototypePush(this[kDirOperationQueue], () => { - this[kDirReadImpl](maybeSync, callback); + if (this.#dirOperationQueue !== null) { + ArrayPrototypePush(this.#dirOperationQueue, () => { + this.#dirReadImpl(maybeSync, callback); }); return; } - if (this[kDirBufferedEntries].length > 0) { + if (this.#dirBufferedEntries.length > 0) { try { - const dirent = ArrayPrototypeShift(this[kDirBufferedEntries]); + const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); - if (this[kDirOptions].recursive && dirent.isDirectory()) { + if (this.#dirOptions.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } @@ -118,8 +116,8 @@ class Dir { const req = new FSReqCallback(); req.oncomplete = (err, result) => { process.nextTick(() => { - const queue = this[kDirOperationQueue]; - this[kDirOperationQueue] = null; + const queue = this.#dirOperationQueue; + this.#dirOperationQueue = null; for (const op of queue) op(); }); @@ -128,9 +126,9 @@ class Dir { } try { - this.processReadResult(this[kDirPath], result); - const dirent = ArrayPrototypeShift(this[kDirBufferedEntries]); - if (this[kDirOptions].recursive && dirent.isDirectory()) { + this.processReadResult(this.#dirPath, result); + const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); + if (this.#dirOptions.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } callback(null, dirent); @@ -139,10 +137,10 @@ class Dir { } }; - this[kDirOperationQueue] = []; - this[kDirHandle].read( - this[kDirOptions].encoding, - this[kDirOptions].bufferSize, + this.#dirOperationQueue = []; + this.#dirHandle.read( + this.#dirOptions.encoding, + this.#dirOptions.bufferSize, req, ); } @@ -150,7 +148,7 @@ class Dir { processReadResult(path, result) { for (let i = 0; i < result.length; i += 2) { ArrayPrototypePush( - this[kDirBufferedEntries], + this.#dirBufferedEntries, getDirent( path, result[i], @@ -165,14 +163,14 @@ class Dir { const ctx = { path }; const handle = dirBinding.opendir( pathModule.toNamespacedPath(path), - this[kDirOptions].encoding, + this.#dirOptions.encoding, undefined, ctx, ); handleErrorFromBinding(ctx); const result = handle.read( - this[kDirOptions].encoding, - this[kDirOptions].bufferSize, + this.#dirOptions.encoding, + this.#dirOptions.bufferSize, undefined, ctx, ); @@ -186,26 +184,26 @@ class Dir { } readSync() { - if (this[kDirClosed] === true) { + if (this.#dirClosed === true) { throw new ERR_DIR_CLOSED(); } - if (this[kDirOperationQueue] !== null) { + if (this.#dirOperationQueue !== null) { throw new ERR_DIR_CONCURRENT_OPERATION(); } - if (this[kDirBufferedEntries].length > 0) { - const dirent = ArrayPrototypeShift(this[kDirBufferedEntries]); - if (this[kDirOptions].recursive && dirent.isDirectory()) { + if (this.#dirBufferedEntries.length > 0) { + const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); + if (this.#dirOptions.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } return dirent; } - const ctx = { path: this[kDirPath] }; - const result = this[kDirHandle].read( - this[kDirOptions].encoding, - this[kDirOptions].bufferSize, + const ctx = { path: this.#dirPath }; + const result = this.#dirHandle.read( + this.#dirOptions.encoding, + this.#dirOptions.bufferSize, undefined, ctx, ); @@ -215,10 +213,10 @@ class Dir { return result; } - this.processReadResult(this[kDirPath], result); + this.processReadResult(this.#dirPath, result); - const dirent = ArrayPrototypeShift(this[kDirBufferedEntries]); - if (this[kDirOptions].recursive && dirent.isDirectory()) { + const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); + if (this.#dirOptions.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } return dirent; @@ -227,45 +225,45 @@ class Dir { close(callback) { // Promise if (callback === undefined) { - if (this[kDirClosed] === true) { + if (this.#dirClosed === true) { return PromiseReject(new ERR_DIR_CLOSED()); } - return this[kDirClosePromisified](); + return this.#dirClosePromisified(); } // callback validateFunction(callback, 'callback'); - if (this[kDirClosed] === true) { + if (this.#dirClosed === true) { process.nextTick(callback, new ERR_DIR_CLOSED()); return; } - if (this[kDirOperationQueue] !== null) { - ArrayPrototypePush(this[kDirOperationQueue], () => { + if (this.#dirOperationQueue !== null) { + ArrayPrototypePush(this.#dirOperationQueue, () => { this.close(callback); }); return; } - this[kDirClosed] = true; + this.#dirClosed = true; const req = new FSReqCallback(); req.oncomplete = callback; - this[kDirHandle].close(req); + this.#dirHandle.close(req); } closeSync() { - if (this[kDirClosed] === true) { + if (this.#dirClosed === true) { throw new ERR_DIR_CLOSED(); } - if (this[kDirOperationQueue] !== null) { + if (this.#dirOperationQueue !== null) { throw new ERR_DIR_CONCURRENT_OPERATION(); } - this[kDirClosed] = true; - const ctx = { path: this[kDirPath] }; - const result = this[kDirHandle].close(undefined, ctx); + this.#dirClosed = true; + const ctx = { path: this.#dirPath }; + const result = this.#dirHandle.close(undefined, ctx); handleErrorFromBinding(ctx); return result; } @@ -273,14 +271,14 @@ class Dir { async* entries() { try { while (true) { - const result = await this[kDirReadPromisified](); + const result = await this.#dirReadPromisified(); if (result === null) { break; } yield result; } } finally { - await this[kDirClosePromisified](); + await this.#dirClosePromisified(); } } } From 9af29beef6d6e683d19b56ab32c380b30119a049 Mon Sep 17 00:00:00 2001 From: pluris Date: Mon, 4 Dec 2023 09:30:32 +0900 Subject: [PATCH 2/2] fixup! fs: use private fields instead of symbols for `Dir` --- lib/internal/fs/dir.js | 139 ++++++++++++++++++++--------------------- 1 file changed, 67 insertions(+), 72 deletions(-) diff --git a/lib/internal/fs/dir.js b/lib/internal/fs/dir.js index 761e0f546909af..7a7220815dc374 100644 --- a/lib/internal/fs/dir.js +++ b/lib/internal/fs/dir.js @@ -34,72 +34,67 @@ const { } = require('internal/validators'); class Dir { - #dirHandle; - #dirPath; - #dirBufferedEntries; - #dirClosed; - #dirOptions; - #dirReadPromisified; - #dirClosePromisified; - #dirOperationQueue; + #handle; + #path; + #bufferedEntries = []; + #closed = false; + #options; + #readPromisified; + #closePromisified; + // Either `null` or an Array of pending operations (= functions to be called + // once the current operation is done). + #operationQueue = null; constructor(handle, path, options) { if (handle == null) throw new ERR_MISSING_ARGS('handle'); - this.#dirHandle = handle; - this.#dirBufferedEntries = []; - this.#dirPath = path; - this.#dirClosed = false; - - // Either `null` or an Array of pending operations (= functions to be called - // once the current operation is done). - this.#dirOperationQueue = null; - - this.#dirOptions = { + this.#handle = handle; + this.#path = path; + this.#options = { bufferSize: 32, ...getOptions(options, { encoding: 'utf8', }), }; - validateUint32(this.#dirOptions.bufferSize, 'options.bufferSize', true); + validateUint32(this.#options.bufferSize, 'options.bufferSize', true); - this.#dirReadPromisified = FunctionPrototypeBind( - internalUtil.promisify(this.#dirReadImpl), this, false); - this.#dirClosePromisified = FunctionPrototypeBind( + this.#readPromisified = FunctionPrototypeBind( + internalUtil.promisify(this.#readImpl), this, false); + this.#closePromisified = FunctionPrototypeBind( internalUtil.promisify(this.close), this); } get path() { - return this.#dirPath; + return this.#path; } read(callback) { - return this.#dirReadImpl(true, callback); + return this.#readImpl(true, callback); } - #dirReadImpl(maybeSync, callback) { - if (this.#dirClosed === true) { + #readImpl(maybeSync, callback) { + if (this.#closed === true) { throw new ERR_DIR_CLOSED(); } if (callback === undefined) { - return this.#dirReadPromisified(); + return this.#readPromisified(); } validateFunction(callback, 'callback'); - if (this.#dirOperationQueue !== null) { - ArrayPrototypePush(this.#dirOperationQueue, () => { - this.#dirReadImpl(maybeSync, callback); + if (this.#operationQueue !== null) { + ArrayPrototypePush(this.#operationQueue, () => { + this.#readImpl(maybeSync, callback); }); return; } - if (this.#dirBufferedEntries.length > 0) { + if (this.#bufferedEntries.length > 0) { try { - const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); + const dirent = ArrayPrototypeShift(this.#bufferedEntries); - if (this.#dirOptions.recursive && dirent.isDirectory()) { + if (this.#options.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } @@ -116,8 +111,8 @@ class Dir { const req = new FSReqCallback(); req.oncomplete = (err, result) => { process.nextTick(() => { - const queue = this.#dirOperationQueue; - this.#dirOperationQueue = null; + const queue = this.#operationQueue; + this.#operationQueue = null; for (const op of queue) op(); }); @@ -126,9 +121,9 @@ class Dir { } try { - this.processReadResult(this.#dirPath, result); - const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); - if (this.#dirOptions.recursive && dirent.isDirectory()) { + this.processReadResult(this.#path, result); + const dirent = ArrayPrototypeShift(this.#bufferedEntries); + if (this.#options.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } callback(null, dirent); @@ -137,10 +132,10 @@ class Dir { } }; - this.#dirOperationQueue = []; - this.#dirHandle.read( - this.#dirOptions.encoding, - this.#dirOptions.bufferSize, + this.#operationQueue = []; + this.#handle.read( + this.#options.encoding, + this.#options.bufferSize, req, ); } @@ -148,7 +143,7 @@ class Dir { processReadResult(path, result) { for (let i = 0; i < result.length; i += 2) { ArrayPrototypePush( - this.#dirBufferedEntries, + this.#bufferedEntries, getDirent( path, result[i], @@ -163,14 +158,14 @@ class Dir { const ctx = { path }; const handle = dirBinding.opendir( pathModule.toNamespacedPath(path), - this.#dirOptions.encoding, + this.#options.encoding, undefined, ctx, ); handleErrorFromBinding(ctx); const result = handle.read( - this.#dirOptions.encoding, - this.#dirOptions.bufferSize, + this.#options.encoding, + this.#options.bufferSize, undefined, ctx, ); @@ -184,26 +179,26 @@ class Dir { } readSync() { - if (this.#dirClosed === true) { + if (this.#closed === true) { throw new ERR_DIR_CLOSED(); } - if (this.#dirOperationQueue !== null) { + if (this.#operationQueue !== null) { throw new ERR_DIR_CONCURRENT_OPERATION(); } - if (this.#dirBufferedEntries.length > 0) { - const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); - if (this.#dirOptions.recursive && dirent.isDirectory()) { + if (this.#bufferedEntries.length > 0) { + const dirent = ArrayPrototypeShift(this.#bufferedEntries); + if (this.#options.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } return dirent; } - const ctx = { path: this.#dirPath }; - const result = this.#dirHandle.read( - this.#dirOptions.encoding, - this.#dirOptions.bufferSize, + const ctx = { path: this.#path }; + const result = this.#handle.read( + this.#options.encoding, + this.#options.bufferSize, undefined, ctx, ); @@ -213,10 +208,10 @@ class Dir { return result; } - this.processReadResult(this.#dirPath, result); + this.processReadResult(this.#path, result); - const dirent = ArrayPrototypeShift(this.#dirBufferedEntries); - if (this.#dirOptions.recursive && dirent.isDirectory()) { + const dirent = ArrayPrototypeShift(this.#bufferedEntries); + if (this.#options.recursive && dirent.isDirectory()) { this.readSyncRecursive(dirent); } return dirent; @@ -225,45 +220,45 @@ class Dir { close(callback) { // Promise if (callback === undefined) { - if (this.#dirClosed === true) { + if (this.#closed === true) { return PromiseReject(new ERR_DIR_CLOSED()); } - return this.#dirClosePromisified(); + return this.#closePromisified(); } // callback validateFunction(callback, 'callback'); - if (this.#dirClosed === true) { + if (this.#closed === true) { process.nextTick(callback, new ERR_DIR_CLOSED()); return; } - if (this.#dirOperationQueue !== null) { - ArrayPrototypePush(this.#dirOperationQueue, () => { + if (this.#operationQueue !== null) { + ArrayPrototypePush(this.#operationQueue, () => { this.close(callback); }); return; } - this.#dirClosed = true; + this.#closed = true; const req = new FSReqCallback(); req.oncomplete = callback; - this.#dirHandle.close(req); + this.#handle.close(req); } closeSync() { - if (this.#dirClosed === true) { + if (this.#closed === true) { throw new ERR_DIR_CLOSED(); } - if (this.#dirOperationQueue !== null) { + if (this.#operationQueue !== null) { throw new ERR_DIR_CONCURRENT_OPERATION(); } - this.#dirClosed = true; - const ctx = { path: this.#dirPath }; - const result = this.#dirHandle.close(undefined, ctx); + this.#closed = true; + const ctx = { path: this.#path }; + const result = this.#handle.close(undefined, ctx); handleErrorFromBinding(ctx); return result; } @@ -271,14 +266,14 @@ class Dir { async* entries() { try { while (true) { - const result = await this.#dirReadPromisified(); + const result = await this.#readPromisified(); if (result === null) { break; } yield result; } } finally { - await this.#dirClosePromisified(); + await this.#closePromisified(); } } }