Change the way the MultiItemCache iterates over items to prevent stack exhaustion when it has a large number of items.

dtanders · dtanders · commit 9c6557e8ec9d · 2021-12-23T11:24:59.000-06:00
Added unit tests for MultiItemCache behavior, including one for covering the large item count case.
diff --git a/lib/CacheFacade.js b/lib/CacheFacade.js
@@ -46,15 +46,23 @@ class MultiItemCache {
 	 * @returns {void}
 	 */
 	get(callback) {
-		const next = i => {
-			this._items[i].get((err, result) => {
+		let i = 0;
+		let resultFound = false;
+		const generateGetCallback = (/** @type {Boolean} */ lastItem) => {
+			return (
+				/** @type {import("./WebpackError")} */ err,
+				/** @type {T} */ res
+			) => {
+				if (resultFound) return;
+				resultFound = !!(err || res || false);
 				if (err) return callback(err);
-				if (result !== undefined) return callback(null, result);
-				if (++i >= this._items.length) return callback();
-				next(i);
-			});
+				if (res !== undefined) return callback(null, res);
+				if (lastItem) return callback();
+			};
 		};
-		next(0);
+		do {
+			this._items[i].get(generateGetCallback(i === this._items.length - 1));
+		} while (++i < this._items.length);
 	}
 
 	/**
diff --git a/test/MultiItemCache.unittest.js b/test/MultiItemCache.unittest.js
@@ -0,0 +1,65 @@
+"use strict";
+
+const Cache = require("../lib/Cache");
+const { ItemCacheFacade, MultiItemCache } = require("../lib/CacheFacade");
+
+describe("MultiItemCache", () => {
+	it("Throws when getting items from an empty Cache", () => {
+		const multiItemCache = new MultiItemCache(generateItemCaches(0));
+		expect(() => multiItemCache.get(_ => _())).toThrowError();
+	});
+
+	it("Returns the single ItemCacheFacade when passed an array of length 1", () => {
+		const itemCaches = generateItemCaches(1);
+		const multiItemCache = new MultiItemCache(itemCaches);
+		expect(multiItemCache).toBe(itemCaches[0]);
+	});
+
+	it("Retrieves items from the underlying Cache when get is called", () => {
+		const itemCaches = generateItemCaches(10);
+		const multiItemCache = new MultiItemCache(itemCaches);
+		const callback = (err, res) => {
+			expect(err).toBeNull();
+			expect(res).toBeInstanceOf(Object);
+		};
+		for (let i = 0; i < 10; ++i) {
+			multiItemCache.get(callback);
+		}
+	});
+
+	it("Can get() a large number of items without exhausting the stack", () => {
+		const itemCaches = generateItemCaches(10000, () => undefined);
+		const multiItemCache = new MultiItemCache(itemCaches);
+		let callbacks = 0;
+		const callback = (err, res) => {
+			expect(err).toBeUndefined();
+			expect(res).toBeUndefined();
+			++callbacks;
+		};
+		multiItemCache.get(callback);
+		expect(callbacks).toEqual(1);
+	});
+
+	function generateItemCaches(howMany, dataGenerator) {
+		const ret = [];
+		for (let i = 0; i < howMany; ++i) {
+			const name = `ItemCache${i}`;
+			const tag = `ItemTag${i}`;
+			const dataGen =
+				dataGenerator ||
+				(() => {
+					return { name: tag };
+				});
+			const cache = new Cache();
+			cache.hooks.get.tapAsync(
+				"DataReturner",
+				(_identifier, _etag, _gotHandlers, callback) => {
+					callback(undefined, dataGen());
+				}
+			);
+			const itemCache = new ItemCacheFacade(cache, name, tag);
+			ret[i] = itemCache;
+		}
+		return ret;
+	}
+});
