From 24c61a763d54d76057e551be88a9ec88d610fb18 Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 17 Oct 2024 16:07:15 -0400 Subject: [PATCH 1/2] Prevent stack overflow for deep nested promise chain --- package-lock.json | 44 ++++++++++++++++++------------------- src/source/promises.bs | 8 ++++++- src/source/promises.spec.bs | 33 +++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 60ed3fb..e3a261d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -407,12 +407,12 @@ } }, "node_modules/braces": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz", - "integrity": "sha512-b8um+L1RzM3WDSzvhm6gIz1yfTbBt6YTlcEKAvsmqCZZFw46z626lVj9j1yEPW33H5H+lBQpZMP1k8l+78Ha0A==", + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.3.tgz", + "integrity": "sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA==", "dev": true, "dependencies": { - "fill-range": "^7.0.1" + "fill-range": "^7.1.1" }, "engines": { "node": ">=8" @@ -789,9 +789,9 @@ } }, "node_modules/fill-range": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.0.1.tgz", - "integrity": "sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==", + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.1.1.tgz", + "integrity": "sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg==", "dev": true, "dependencies": { "to-regex-range": "^5.0.1" @@ -1174,12 +1174,12 @@ } }, "node_modules/micromatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", - "integrity": "sha512-DMy+ERcEW2q8Z2Po+WNXuw3c5YaUSFjAO5GsJqfEl7UjvtIuFKO6ZrKvcItdy98dwFI2N1tg3zNIdKaQT+aNdA==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "dependencies": { - "braces": "^3.0.2", + "braces": "^3.0.3", "picomatch": "^2.3.1" }, "engines": { @@ -2410,12 +2410,12 @@ } }, "braces": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz", - "integrity": "sha512-b8um+L1RzM3WDSzvhm6gIz1yfTbBt6YTlcEKAvsmqCZZFw46z626lVj9j1yEPW33H5H+lBQpZMP1k8l+78Ha0A==", + "version": "3.0.3", + "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.3.tgz", + "integrity": "sha512-yQbXgO/OSZVD2IsiLlro+7Hf6Q18EJrKSEsdoMzKePKXct3gvD8oLcOQdIzGupr5Fj+EDe8gO/lxc1BzfMpxvA==", "dev": true, "requires": { - "fill-range": "^7.0.1" + "fill-range": "^7.1.1" } }, "brighterscript": { @@ -2732,9 +2732,9 @@ "dev": true }, "fill-range": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.0.1.tgz", - "integrity": "sha512-qOo9F+dMUmC2Lcb4BbVvnKJxTPjCm+RRpe4gDuGrzkL7mEVl/djYSu2OdQ2Pa302N4oqkSg9ir6jaLWJ2USVpQ==", + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/fill-range/-/fill-range-7.1.1.tgz", + "integrity": "sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg==", "dev": true, "requires": { "to-regex-range": "^5.0.1" @@ -3040,12 +3040,12 @@ "dev": true }, "micromatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", - "integrity": "sha512-DMy+ERcEW2q8Z2Po+WNXuw3c5YaUSFjAO5GsJqfEl7UjvtIuFKO6ZrKvcItdy98dwFI2N1tg3zNIdKaQT+aNdA==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "requires": { - "braces": "^3.0.2", + "braces": "^3.0.3", "picomatch": "^2.3.1" } }, diff --git a/src/source/promises.bs b/src/source/promises.bs index a4f5d9e..d9f1eb0 100644 --- a/src/source/promises.bs +++ b/src/source/promises.bs @@ -251,7 +251,13 @@ namespace promises.internal if storage = invalid then ' unregister any observers on the promise to prevent multiple callbacks promises.internal.unobserveFieldScoped(promise, promises.internal.PromiseField.promiseState) - promises.internal.observeFieldScoped(promise, promises.internal.PromiseField.promiseState, promises.internal.notifyListeners) + promises.internal.observeFieldScoped(promise, promises.internal.PromiseField.promiseState, sub(event) + 'run the notification nexttick to prevent stackoverflow due to cascading promises all resolving in sequence + promises.internal.delay(sub(context) + promises.internal.notifyListeners(context.event) + end sub, { event: event }) + end sub) + storage = { promise: promise thenListeners: [] diff --git a/src/source/promises.spec.bs b/src/source/promises.spec.bs index f014da6..7c05ffd 100644 --- a/src/source/promises.spec.bs +++ b/src/source/promises.spec.bs @@ -115,7 +115,7 @@ namespace tests promises.onThen(promise, sub(_ as dynamic, ctx as dynamic) elapsedTimeInMillis = ctx.timespan.totalMilliseconds() ? "elapsed time to resolve promise:" + elapsedTimeInMillis.tostr() - tolerance = ctx.timerDurationInMillis * 0.1 + tolerance = ctx.timerDurationInMillis * 0.2 msg = "did not settle within 10% tolerance of timer duration" m.testSuite.assertTrue(ctx.timerDurationInMillis - tolerance <= elapsedTimeInMillis, msg) m.testSuite.assertTrue(ctx.timerDurationInMillis + tolerance >= elapsedTimeInMillis, msg) @@ -352,6 +352,37 @@ namespace tests end sub, testNode) end function + @async(60000) + @it("unravels deep promise chain without crashing due to stackoverflow") + function _() + 'this function creates a promise that depends on another promise (until we hit a max) + doWork = function(context) + 'if we hit the max, resolve the promise and unravel the entire stack + if context.currentCount > 10000 + return promises.resolve(true) + end if + context.currentCount = context.currentCount + 1 + + 'return a promise that depends on another future promise + return promises.onThen(promises.resolve(true), function(result, context) + doWork = context.doWork + return doWork(context) + end function, context) + end function + + promises.chain(promises.resolve(true), { + currentCount: 0, + doWork: doWork + }).then(function(result, context) + doWork = context.doWork + return doWork(context) + end function).then(function(result, context) + m.testSuite.done() + end function).catch(function(error, context) + print "error", error, FormatJson(error.backtrace) + end function).toPromise() + + end function end class end namespace From b07c2a5c898bad9b20a5a05c2490b3e8ed8e9d5d Mon Sep 17 00:00:00 2001 From: Bronley Plumb Date: Thu, 17 Oct 2024 16:07:38 -0400 Subject: [PATCH 2/2] Delete the timer key instead of settng to invalid --- src/source/promises.bs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/source/promises.bs b/src/source/promises.bs index d9f1eb0..c0ab356 100644 --- a/src/source/promises.bs +++ b/src/source/promises.bs @@ -488,7 +488,7 @@ namespace promises.internal print "Crash during utils.delay:", e #end if end try - m[delayId] = invalid + m.delete(delayId) end sub) timer.control = "start"