Skip to content

Commit

Permalink
worker: reset Isolate stack limit after entering Locker
Browse files Browse the repository at this point in the history
It turns out that using `v8::Locker` undoes the effects of
passing an explicit stack limit as part of the `Isolate`’s
resource constraints.

Therefore, reset the stack limit manually after entering a Locker.

Refs: #26049 (comment)

PR-URL: #31593
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
addaleax authored and codebytere committed Mar 30, 2020
1 parent bdd1133 commit 0697f65
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ class WorkerThreadData {
{
Locker locker(isolate);
Isolate::Scope isolate_scope(isolate);
// V8 computes its stack limit the first time a `Locker` is used based on
// --stack-size. Reset it to the correct value.
isolate->SetStackLimit(w->stack_base_);

HandleScope handle_scope(isolate);
isolate_data_.reset(CreateIsolateData(isolate,
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-worker-stack-overflow-stack-size.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { once } = require('events');
const v8 = require('v8');
const { Worker } = require('worker_threads');

// Verify that Workers don't care about --stack-size, as they have their own
// fixed and known stack sizes.

async function runWorker() {
const empiricalStackDepth = new Uint32Array(new SharedArrayBuffer(4));
const worker = new Worker(`
const { workerData: { empiricalStackDepth } } = require('worker_threads');
function f() {
empiricalStackDepth[0]++;
f();
}
f();`, {
eval: true,
workerData: { empiricalStackDepth }
});

const [ error ] = await once(worker, 'error');

common.expectsError({
constructor: RangeError,
message: 'Maximum call stack size exceeded'
})(error);

return empiricalStackDepth[0];
}

(async function() {
v8.setFlagsFromString('--stack-size=500');
const w1stack = await runWorker();
v8.setFlagsFromString('--stack-size=1000');
const w2stack = await runWorker();
// Make sure the two stack sizes are within 10 % of each other, i.e. not
// affected by the different `--stack-size` settings.
assert(Math.max(w1stack, w2stack) / Math.min(w1stack, w2stack) < 1.1,
`w1stack = ${w1stack}, w2stack ${w2stack} are too far apart`);
})().then(common.mustCall());

0 comments on commit 0697f65

Please sign in to comment.