From 79163baaf93697505aaa0926d6466e0d0156761c Mon Sep 17 00:00:00 2001
From: Tobias Koppers <tobias.koppers@googlemail.com>
Date: Fri, 12 Nov 2021 12:30:42 +0100
Subject: [PATCH 1/2] use a faster readFile when file size is already in cache
---
lib/CachedInputFileSystem.js | 232 ++++++++++++++++++++++++++++++++---
lib/Resolver.js | 12 +-
types.d.ts | 44 ++++++-
3 files changed, 263 insertions(+), 25 deletions(-)
diff --git a/lib/CachedInputFileSystem.js b/lib/CachedInputFileSystem.js
index 74dbd982..d5347019 100644
--- a/lib/CachedInputFileSystem.js
+++ b/lib/CachedInputFileSystem.js
@@ -47,44 +47,56 @@ class OperationMergerBackend {
* @param {any} providerContext call context for the provider methods
*/
constructor(provider, syncProvider, providerContext) {
- this._provider = provider;
- this._syncProvider = syncProvider;
- this._providerContext = providerContext;
- this._activeAsyncOperations = new Map();
+ const activeAsyncOperations = new Map();
- this.provide = this._provider
+ this.provide = provider
? (path, options, callback) => {
if (typeof options === "function") {
callback = options;
options = undefined;
}
if (options) {
- return this._provider.call(
- this._providerContext,
- path,
- options,
- callback
- );
+ return provider.call(providerContext, path, options, callback);
}
if (typeof path !== "string") {
callback(new TypeError("path must be a string"));
return;
}
- let callbacks = this._activeAsyncOperations.get(path);
+ let callbacks = activeAsyncOperations.get(path);
if (callbacks) {
callbacks.push(callback);
return;
}
- this._activeAsyncOperations.set(path, (callbacks = [callback]));
- provider(path, (err, result) => {
- this._activeAsyncOperations.delete(path);
+ activeAsyncOperations.set(path, (callbacks = [callback]));
+ provider.call(providerContext, path, (err, result) => {
+ activeAsyncOperations.delete(path);
runCallbacks(callbacks, err, result);
});
}
: null;
- this.provideSync = this._syncProvider
+ this.provideSync = syncProvider
? (path, options) => {
- return this._syncProvider.call(this._providerContext, path, options);
+ return syncProvider.call(providerContext, path, options);
+ }
+ : null;
+
+ this.provideCustom = provider
+ ? (provider, providerContext, path, callback) => {
+ let callbacks = activeAsyncOperations.get(path);
+ if (callbacks) {
+ callbacks.push(callback);
+ return;
+ }
+ activeAsyncOperations.set(path, (callbacks = [callback]));
+ provider.call(providerContext, path, (err, result) => {
+ activeAsyncOperations.delete(path);
+ runCallbacks(callbacks, err, result);
+ });
+ }
+ : null;
+ this.provideCustomSync = syncProvider
+ ? (syncProvider, providerContext, path) => {
+ return syncProvider.call(providerContext, path);
}
: null;
}
@@ -149,6 +161,11 @@ class CacheBackend {
this.provide = provider ? this.provide.bind(this) : null;
this.provideSync = syncProvider ? this.provideSync.bind(this) : null;
+
+ this.provideCustom = provider ? this.provideCustom.bind(this) : null;
+ this.provideCustomSync = syncProvider
+ ? this.provideCustomSync.bind(this)
+ : null;
}
provide(path, options, callback) {
@@ -243,6 +260,74 @@ class CacheBackend {
return result;
}
+ provideCustom(provider, providerContext, path, callback) {
+ // When in sync mode we can move to async mode
+ if (this._mode === STORAGE_MODE_SYNC) {
+ this._enterAsyncMode();
+ }
+
+ // Check in cache
+ let cacheEntry = this._data.get(path);
+ if (cacheEntry !== undefined) {
+ if (cacheEntry.err) return nextTick(callback, cacheEntry.err);
+ return nextTick(callback, null, cacheEntry.result);
+ }
+
+ // Check if there is already the same operation running
+ let callbacks = this._activeAsyncOperations.get(path);
+ if (callbacks !== undefined) {
+ callbacks.push(callback);
+ return;
+ }
+ this._activeAsyncOperations.set(path, (callbacks = [callback]));
+
+ // Run the operation
+ provider.call(providerContext, path, (err, result) => {
+ this._activeAsyncOperations.delete(path);
+ this._storeResult(path, err, result);
+
+ // Enter async mode if not yet done
+ this._enterAsyncMode();
+
+ runCallbacks(callbacks, err, result);
+ });
+ }
+
+ provideCustomSync(provider, providerContext, path) {
+ // In sync mode we may have to decay some cache items
+ if (this._mode === STORAGE_MODE_SYNC) {
+ this._runDecays();
+ }
+
+ // Check in cache
+ let cacheEntry = this._data.get(path);
+ if (cacheEntry !== undefined) {
+ if (cacheEntry.err) throw cacheEntry.err;
+ return cacheEntry.result;
+ }
+
+ // Get all active async operations
+ // This sync operation will also complete them
+ const callbacks = this._activeAsyncOperations.get(path);
+ this._activeAsyncOperations.delete(path);
+
+ // Run the operation
+ // When in idle mode, we will enter sync mode
+ let result;
+ try {
+ result = provider.call(providerContext, path);
+ } catch (err) {
+ this._storeResult(path, err, undefined);
+ this._enterSyncModeWhenIdle();
+ if (callbacks) runCallbacks(callbacks, err, undefined);
+ throw err;
+ }
+ this._storeResult(path, undefined, result);
+ this._enterSyncModeWhenIdle();
+ if (callbacks) runCallbacks(callbacks, undefined, result);
+ return result;
+ }
+
purge(what) {
if (!what) {
if (this._mode !== STORAGE_MODE_IDLE) {
@@ -395,8 +480,10 @@ module.exports = class CachedInputFileSystem {
this.fileSystem
);
const stat = this._statBackend.provide;
+ const customStat = this._statBackend.provideCustom;
this.stat = /** @type {FileSystem["stat"]} */ (stat);
const statSync = this._statBackend.provideSync;
+ const customStatSync = this._statBackend.provideCustomSync;
this.statSync = /** @type {SyncFileSystem["statSync"]} */ (statSync);
this._readdirBackend = createBackend(
@@ -412,8 +499,115 @@ module.exports = class CachedInputFileSystem {
this._readFileBackend = createBackend(
duration,
- this.fileSystem.readFile,
- this.fileSystem.readFileSync,
+ this.fileSystem.readFile &&
+ this.fileSystem.fstat &&
+ this.fileSystem.read &&
+ this.fileSystem.open &&
+ this.fileSystem.close &&
+ customStat
+ ? /**
+ * @this {{ fstat: NonNullable<FileSystem["fstat"]>, readFile: NonNullable<FileSystem["readFile"]>, open: NonNullable<FileSystem["open"]>, read: NonNullable<FileSystem["read"]>, close: NonNullable<FileSystem["close"]> }}
+ */
+ function (path, options, callback) {
+ if (typeof options === "function") {
+ callback = options;
+ options = undefined;
+ }
+ if (typeof options === "object")
+ return this.readFile(path, options, callback);
+ this.open(path, "r", (err, fd) => {
+ if (err) return callback(err);
+ if (typeof fd !== "number")
+ return callback(new Error("fd must be a number"));
+ customStat(
+ (path, callback) => this.fstat(fd, callback),
+ null,
+ path,
+ (err, stats) => {
+ if (err) return callback(err);
+ if (stats.size > 0 && stats.size < 128 * 1024) {
+ let remaining = stats.size;
+ const buffer = Buffer.allocUnsafe(remaining);
+ const afterRead = (err, bytesRead) => {
+ if (err) {
+ return this.close(fd, () => {
+ callback(err);
+ });
+ }
+ remaining -= bytesRead;
+ if (bytesRead === 0 || remaining === 0) {
+ this.close(fd, err => {
+ if (err) return callback(err);
+ return callback(null, buffer);
+ });
+ } else {
+ this.read(
+ fd,
+ buffer,
+ stats.size - remaining,
+ remaining,
+ -1,
+ afterRead
+ );
+ }
+ };
+ this.read(fd, buffer, 0, remaining, -1, afterRead);
+ } else {
+ this.readFile(fd, options, (err, buffer) => {
+ this.close(fd, closeErr => {
+ if (err) return callback(err);
+ if (closeErr) return callback(closeErr);
+ callback(null, buffer);
+ });
+ });
+ }
+ }
+ );
+ });
+ }
+ : this.fileSystem.readFile,
+ this.fileSystem.readFileSync &&
+ this.fileSystem.fstatSync &&
+ this.fileSystem.readSync &&
+ this.fileSystem.openSync &&
+ this.fileSystem.closeSync &&
+ customStatSync
+ ? /**
+ * @this {{ fstatSync: NonNullable<SyncFileSystem["fstatSync"]>, readFileSync: NonNullable<SyncFileSystem["readFileSync"]>, openSync: NonNullable<SyncFileSystem["openSync"]>, readSync: NonNullable<SyncFileSystem["readSync"]>, closeSync: NonNullable<SyncFileSystem["closeSync"]> }}
+ */
+ function (path, options) {
+ if (typeof options === "object")
+ return this.readFileSync(path, options);
+ const fd = this.openSync(path, "r");
+ if (typeof fd !== "number") throw new Error("fd must be a number");
+ const stats = customStatSync(() => this.fstatSync(fd), null, path);
+ if (stats.size > 0 && stats.size < 128 * 1024) {
+ let remaining = stats.size;
+ const buffer = Buffer.allocUnsafe(remaining);
+ try {
+ let bytesRead = this.readSync(fd, buffer, 0, remaining, -1);
+ remaining -= bytesRead;
+ while (bytesRead !== 0 && remaining !== 0) {
+ bytesRead = this.readSync(
+ fd,
+ buffer,
+ stats.size - remaining,
+ remaining,
+ -1
+ );
+ remaining -= bytesRead;
+ }
+ return buffer;
+ } finally {
+ this.closeSync(fd);
+ }
+ } else {
+ const buffer = this.readFileSync(fd);
+ this.closeSync(fd);
+ return buffer;
+ }
+ }
+ : this.fileSystem.readFileSync,
this.fileSystem
);
const readFile = this._readFileBackend.provide;
diff --git a/lib/Resolver.js b/lib/Resolver.js
index a3120081..610a3329 100644
--- a/lib/Resolver.js
+++ b/lib/Resolver.js
@@ -47,7 +47,11 @@ const {
/**
* @typedef {Object} FileSystem
- * @property {(function(string, FileSystemCallback<Buffer | string>): void) & function(string, object, FileSystemCallback<Buffer | string>): void} readFile
+ * @property {(function(string | number, FileSystemCallback<Buffer | string>): void) & function(string | number, object, FileSystemCallback<Buffer | string>): void} readFile
+ * @property {(function(string, string | number, FileSystemCallback<number>): void) & function(string, string | number, string | number, FileSystemCallback<number>): void=} open
+ * @property {(function(number, FileSystemCallback<FileSystemStats>): void) & function(number, object, FileSystemCallback<Buffer | string>): void=} fstat
+ * @property {function(number, Buffer | Uint8Array, number, number, number, function(PossibleFileSystemError & Error | null | undefined, number=): void): void=} read
+ * @property {function(number, function(PossibleFileSystemError & Error | null | undefined): void): void=} close
* @property {(function(string, FileSystemCallback<(Buffer | string)[] | FileSystemDirent[]>): void) & function(string, object, FileSystemCallback<(Buffer | string)[] | FileSystemDirent[]>): void} readdir
* @property {((function(string, FileSystemCallback<object>): void) & function(string, object, FileSystemCallback<object>): void)=} readJson
* @property {(function(string, FileSystemCallback<Buffer | string>): void) & function(string, object, FileSystemCallback<Buffer | string>): void} readlink
@@ -57,7 +61,11 @@ const {
/**
* @typedef {Object} SyncFileSystem
- * @property {function(string, object=): Buffer | string} readFileSync
+ * @property {function(string | number, object=): Buffer | string} readFileSync
+ * @property {function(string, string | number, string | number=): number=} openSync
+ * @property {function(number, object=): FileSystemStats=} fstatSync
+ * @property {function(number, Buffer | Uint8Array, number, number, number): number=} readSync
+ * @property {function(number): void=} closeSync
* @property {function(string, object=): (Buffer | string)[] | FileSystemDirent[]} readdirSync
* @property {(function(string, object=): object)=} readJsonSync
* @property {function(string, object=): Buffer | string} readlinkSync
diff --git a/types.d.ts b/types.d.ts
index f392bd1c..0212a5c3 100644
--- a/types.d.ts
+++ b/types.d.ts
@@ -61,14 +61,14 @@ declare class CachedInputFileSystem {
arg1?: object
) => (string | Buffer)[] | FileSystemDirent[];
readFile: {
- (arg0: string, arg1: FileSystemCallback<string | Buffer>): void;
+ (arg0: string | number, arg1: FileSystemCallback<string | Buffer>): void;
(
- arg0: string,
+ arg0: string | number,
arg1: object,
arg2: FileSystemCallback<string | Buffer>
): void;
};
- readFileSync: (arg0: string, arg1?: object) => string | Buffer;
+ readFileSync: (arg0: string | number, arg1?: object) => string | Buffer;
readJson?: {
(arg0: string, arg1: FileSystemCallback<object>): void;
(arg0: string, arg1: object, arg2: FileSystemCallback<object>): void;
@@ -93,13 +93,49 @@ declare class CloneBasenamePlugin {
}
declare interface FileSystem {
readFile: {
- (arg0: string, arg1: FileSystemCallback<string | Buffer>): void;
+ (arg0: string | number, arg1: FileSystemCallback<string | Buffer>): void;
+ (
+ arg0: string | number,
+ arg1: object,
+ arg2: FileSystemCallback<string | Buffer>
+ ): void;
+ };
+ open?: {
(
arg0: string,
+ arg1: string | number,
+ arg2: FileSystemCallback<number>
+ ): void;
+ (
+ arg0: string,
+ arg1: string | number,
+ arg2: string | number,
+ arg3: FileSystemCallback<number>
+ ): void;
+ };
+ fstat?: {
+ (arg0: number, arg1: FileSystemCallback<FileSystemStats>): void;
+ (
+ arg0: number,
arg1: object,
arg2: FileSystemCallback<string | Buffer>
): void;
};
+ read?: (
+ arg0: number,
+ arg1: Buffer | Uint8Array,
+ arg2: number,
+ arg3: number,
+ arg4: number,
+ arg5: (
+ arg0?: null | (PossibleFileSystemError & Error),
+ arg1?: number
+ ) => void
+ ) => void;
+ close?: (
+ arg0: number,
+ arg1: (arg0?: null | (PossibleFileSystemError & Error)) => void
+ ) => void;
readdir: {
(
arg0: string,
From a7c161eeb141bfc7fda375df36c13df006a8cf76 Mon Sep 17 00:00:00 2001
From: Tobias Koppers <tobias.koppers@googlemail.com>
Date: Mon, 15 Nov 2021 11:17:09 +0100
Subject: [PATCH 2/2] add detection logic for race conditions due to cached
file size
---
lib/CachedInputFileSystem.js | 54 ++++++++++++++++++++++++++++++++---
test/CachedInputFileSystem.js | 53 ++++++++++++++++++++++++++++++++++
2 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/lib/CachedInputFileSystem.js b/lib/CachedInputFileSystem.js
index d5347019..b1573e93 100644
--- a/lib/CachedInputFileSystem.js
+++ b/lib/CachedInputFileSystem.js
@@ -526,7 +526,7 @@ module.exports = class CachedInputFileSystem {
(err, stats) => {
if (err) return callback(err);
if (stats.size > 0 && stats.size < 128 * 1024) {
- let remaining = stats.size;
+ let remaining = stats.size + 1;
const buffer = Buffer.allocUnsafe(remaining);
const afterRead = (err, bytesRead) => {
if (err) {
@@ -535,17 +535,63 @@ module.exports = class CachedInputFileSystem {
});
}
remaining -= bytesRead;
- if (bytesRead === 0 || remaining === 0) {
+ if (bytesRead === 0 || remaining === 1) {
this.close(fd, err => {
if (err) return callback(err);
- return callback(null, buffer);
+ return callback(
+ null,
+ buffer.slice(0, buffer.length - remaining)
+ );
});
+ } else if (remaining === 0) {
+ // The file size has changed from the cached info
+ // We keep reading until the end is found
+ let buf = Buffer.allocUnsafe(16 * 1024);
+ let bufPos = 0;
+ const buffers = [buffer];
+ const afterUnknownRead = (err, bytesRead) => {
+ if (err) {
+ return this.close(fd, () => {
+ callback(err);
+ });
+ }
+ bufPos += bytesRead;
+ if (bytesRead === 0) {
+ if (bufPos > 0) buffers.push(buf.slice(0, bufPos));
+ this.close(fd, err => {
+ if (err) return callback(err);
+ return callback(null, Buffer.concat(buffers));
+ });
+ } else {
+ if (bufPos === buf.length) {
+ buffers.push(buf);
+ buf = Buffer.allocUnsafe(16 * 1024);
+ bufPos = 0;
+ }
+ this.read(
+ fd,
+ buf,
+ bufPos,
+ buf.length - bufPos,
+ -1,
+ afterUnknownRead
+ );
+ }
+ };
+ this.read(
+ fd,
+ buf,
+ bufPos,
+ buf.length - bufPos,
+ -1,
+ afterUnknownRead
+ );
} else {
this.read(
fd,
buffer,
stats.size - remaining,
- remaining,
+ remaining + 1,
-1,
afterRead
);
diff --git a/test/CachedInputFileSystem.js b/test/CachedInputFileSystem.js
index 21aea5aa..c0caf27f 100644
--- a/test/CachedInputFileSystem.js
+++ b/test/CachedInputFileSystem.js
@@ -364,3 +364,56 @@ describe("CachedInputFileSystem CacheBackend", function () {
next();
});
});
+
+describe("CachedInputFileSystem cached stats", function () {
+ this.timeout(3000);
+ it("should fallback to reading full file when size has changed", done => {
+ let filePos = 0;
+ const fs = new CachedInputFileSystem(
+ {
+ stat: (path, callback) => {
+ callback(null, {
+ size: 100
+ });
+ },
+ readFile: (path, callback) => {
+ callback(new Error("shouldn't be used"));
+ },
+ fstat: (fd, callback) => {
+ callback(new Error("shouldn't be used"));
+ },
+ open: (path, flag, callback) => {
+ path.should.be.eql("/any");
+ flag.should.be.eql("r");
+ filePos = 0;
+ callback(null, 123);
+ },
+ read: (fd, buffer, offset, length, position, callback) => {
+ fd.should.be.eql(123);
+ length = Math.min(length, 32123 - filePos);
+ filePos += length;
+ buffer.fill(1, offset, offset + length);
+ process.nextTick(callback, null, length);
+ },
+ close: (fd, callback) => {
+ fd.should.be.eql(123);
+ callback();
+ }
+ },
+ 10000
+ );
+ fs.stat("/any", (err, stats) => {
+ if (err) return done(err);
+ fs.readFile("/any", (err, buffer) => {
+ if (err) return done(err);
+ if (!buffer || typeof buffer === "string")
+ return done(new Error("no buffer"));
+ buffer.length.should.be.eql(32123);
+ for (let i = 0; i < buffer.length; i++) {
+ buffer[i].should.be.eql(1);
+ }
+ done();
+ });
+ });
+ });
+});