Skip to content

Commit

Permalink
Implement more implementorland spinlocks and timeouts -- and fixed an…
Browse files Browse the repository at this point in the history
… issue where the timeout wasn't being unbound in the case where the custom logic threw an unhandled exception.
  • Loading branch information
mikermcneil committed May 7, 2017
1 parent 523f56b commit d82ea5b
Showing 1 changed file with 49 additions and 10 deletions.
59 changes: 49 additions & 10 deletions lib/private/Deferred.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -295,8 +298,44 @@ Deferred.prototype.exec = function(_cb, handleUncaughtException){
}

});//</self._handleExec>

// 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); }
Expand Down

0 comments on commit d82ea5b

Please sign in to comment.