Skip to content

Commit

Permalink
Using wrapped timer calls to avoid globals being removed on dispose. …
Browse files Browse the repository at this point in the history
…Adding support for legacy sandboxes.
  • Loading branch information
kevinswiber committed Nov 9, 2022
1 parent daee5df commit f2640b1
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 22 deletions.
4 changes: 1 addition & 3 deletions lib/postman-sandbox.js
Expand Up @@ -146,9 +146,7 @@ class PostmanSandbox extends UniversalVM {
this.emit('execution.result.' + id, new Error(BRIDGE_DISCONNECTING_ERROR_MESSAGE));
});

setTimeout(() => {
this.disconnect();
}, 100);
this.disconnect();
}
}

Expand Down
9 changes: 3 additions & 6 deletions lib/sandbox/execute-context.js
Expand Up @@ -50,13 +50,10 @@ module.exports = function (scope, code, execution, console, timers, pmapi, onAss
});

scope.exec(code, function (err) {
// we check if the execution went async by determining the timer queue length at this time
execution.return.async = (timers.queueLength() > 0);

// call this hook to perform any post script execution tasks
legacy.finish(scope, pmapi, onAssertion);

function complete () {
// call this hook to perform any post script execution tasks

// if timers are running, we do not need to proceed with any logic of completing execution.
// instead we wait for timer completion callback to fire
if (execution.return.async) {
Expand All @@ -67,6 +64,6 @@ module.exports = function (scope, code, execution, console, timers, pmapi, onAss
timers.terminate(err);
}

setImmediate(complete);
timers.getWrappedTimer('setImmediate')(complete);
});
};
4 changes: 2 additions & 2 deletions lib/sandbox/execute.js
Expand Up @@ -153,7 +153,7 @@ module.exports = function (bridge, glob) {
execution.return.async = true;
}, function (err, dnd) {
// clear timeout tracking timer
waiting && (waiting = clearTimeout(waiting));
waiting && (waiting = timers.getWrappedTimer('clearTimeout')(waiting));

// do not allow any more timers
if (timers) {
Expand All @@ -180,7 +180,7 @@ module.exports = function (bridge, glob) {

// if a timeout is set, we must ensure that all pending timers are cleared and an execution timeout event is
// triggered.
_.isFinite(options.timeout) && (waiting = setTimeout(function () {
_.isFinite(options.timeout) && (waiting = timers.getWrappedTimer('setTimeout')(function () {
timers.terminate(new Error('sandbox: ' +
(execution.return.async ? 'asynchronous' : 'synchronous') + ' script execution timeout'));
}, options.timeout));
Expand Down
15 changes: 6 additions & 9 deletions lib/sandbox/timers.js
Expand Up @@ -114,7 +114,6 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
total = 0, // accumulator to keep track of total timers
pending = 0, // counters to keep track of running timers
sealed = false, // flag that stops all new timer additions
wentAsync = false,
computeTimerEvents;

// do special handling to enable emulation of immediate timers in hosts that lacks them
Expand Down Expand Up @@ -166,7 +165,7 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
}

if (pending === 0 && computeTimerEvents.started) {
setImmediate(maybeEndAllTimers);
timers.setImmediate(maybeEndAllTimers);
}

if (pending > 0 && !computeTimerEvents.started) {
Expand All @@ -191,8 +190,6 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
args = arrayProtoSlice.call(arguments);

args[0] = function () {
wentAsync = true; // mark that we did go async once. this will ensure we do not pass erroneous events

// call the actual callback with a dummy context
try { callback.apply(dummyContext, staticTimerFunctions[name] ? arguments : null); }
catch (e) { onError && onError(e); }
Expand All @@ -211,9 +208,6 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
}
};

// for static timers
staticTimerFunctions[name] && (wentAsync = true);

// call the underlying timer function and keep a track of its irq
running[id] = timers[('set' + name)].apply(this, args);
args = null; // precaution
Expand Down Expand Up @@ -245,7 +239,7 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
catch (e) { onError(e); }

// decrement counters and call the clearing timer function
computeTimerEvents(-1, !wentAsync);
computeTimerEvents(-1);

args = underLyingId = null; // just a precaution
};
Expand All @@ -272,7 +266,6 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
}
}.bind(this));


/**
* @memberof Timerz.prototype
* @returns {Number}
Expand All @@ -281,6 +274,10 @@ function Timerz (delegations, onError, onAnyTimerStart, onAllTimerEnd) {
return pending;
};

this.getWrappedTimer = function (fnName) {
return timers[fnName];
};

/**
* @memberof Timerz.prototype
*/
Expand Down
4 changes: 2 additions & 2 deletions test/unit/sandbox-timers.test.js
Expand Up @@ -59,7 +59,7 @@ describe('timers inside sandbox', function () {

expect(err).to.be.null;
expect(res).to.nested.include({
'return.async': false
'return.async': true
});

// we wait for a while to ensure that the timeout was actually cleared.
Expand Down Expand Up @@ -114,7 +114,7 @@ describe('timers inside sandbox', function () {

expect(err).to.be.null;
expect(res).to.nested.include({
'return.async': false
'return.async': true
});

// we wait for a while to ensure that the timeout was actually cleared.
Expand Down

0 comments on commit f2640b1

Please sign in to comment.