From d82ea5b0d01d289a53c73b475930925ee2a2992e Mon Sep 17 00:00:00 2001 From: Mike McNeil Date: Sun, 7 May 2017 17:09:17 -0500 Subject: [PATCH] Implement more implementorland spinlocks and timeouts -- and fixed an issue where the timeout wasn't being unbound in the case where the custom logic threw an unhandled exception. --- lib/private/Deferred.js | 59 ++++++++++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/lib/private/Deferred.js b/lib/private/Deferred.js index ad099a0..008d98e 100644 --- a/lib/private/Deferred.js +++ b/lib/private/Deferred.js @@ -228,16 +228,6 @@ Deferred.prototype.exec = function(_cb, handleUncaughtException){ try { self._handleExec(function (err, result) { - // If the deferred has already timed out, then there's no need to warn - // (This was _bound_ to happen beings as how we timed out.) - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - // FUTURE: Maybe keep track to make sure this if/then statement doesn't check - // doesn't come back as true more than once (because that would definitely still - // be unexpected-- and really hard to anticipate / debug if it were to happen to you) - // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - if (self._hasTimedOut) { - return; - } // Implementorland spinlock if (self._hasFinishedExecuting) { @@ -252,11 +242,24 @@ Deferred.prototype.exec = function(_cb, handleUncaughtException){ '```\n'+ (new Error()).stack+'\n'+ '```\n'+ + '\n'+ + 'For more help, visit https://sailsjs.com/support\n'+ '- - - - - - - - - - - - - - - - - - - - - - - - - - - - - -' ); return; } + // If the deferred has already timed out, then there's no need to warn + // (This was _bound_ to happen beings as how we timed out.) + // + // > Note that we still set a flag to track that this happened. This is to make sure + // > this if/then statement can't possibly be true more than once (because that would + // > definitely still be unexpected-- and really hard to anticipate / debug if it were + // > to happen to you) + if (self._hasTimedOut) { + self._hasFinishedExecuting = true; + return; + } // Clear timeout, if relevant. if (timeoutAlarm) { @@ -295,8 +298,44 @@ Deferred.prototype.exec = function(_cb, handleUncaughtException){ } });// + + // Handle errors thrown synchronously by the `_handleExec` implementation: } catch (e) { + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + // NOTE: The following code is ALMOST exactly the same as the code above. + // It's duplicated in place rather than extrapolating purely for performance, + // and since the error messages vary a bit: + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + if (self._hasFinishedExecuting) { + console.warn( + '- - - - - - - - - - - - - - - - - - - - - - - - - - - - - -\n'+ + 'WARNING: Something seems to be wrong with this function.\n'+ + 'It threw an unhandled error during the first tick of its\n'+ + 'execution... but before doing so, it somehow managed to\n'+ + 'have already resolved/rejected once.\n'+ + '(silently ignoring this...)\n'+ + '\n'+ + 'To assist you in hunting this down, here is a stack trace:\n'+ + '```\n'+ + (new Error()).stack+'\n'+ + '```\n'+ + '\n'+ + 'For more help, visit https://sailsjs.com/support\n'+ + '- - - - - - - - - - - - - - - - - - - - - - - - - - - - - -' + ); + return; + }//-• + + if (self._hasTimedOut) { + self._hasFinishedExecuting = true; + return; + }//-• + + if (timeoutAlarm) { + clearTimeout(timeoutAlarm); + }//>- + var err; if (_.isError(e)) { err = e; } else if (_.isString(e)) { err = new Error(e); }