From 1592c474dabc3e9b198b473cc0178e4a8e15d62e Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Fri, 31 Jan 2020 20:06:56 +0100
Subject: [PATCH] worker: reset `Isolate` stack limit after entering `Locker`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/pull/26049#issuecomment-580668530

PR-URL: https://github.com/nodejs/node/pull/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>
---
 src/node_worker.cc                            |  3 ++
 .../test-worker-stack-overflow-stack-size.js  | 43 +++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 test/parallel/test-worker-stack-overflow-stack-size.js

diff --git a/src/node_worker.cc b/src/node_worker.cc
index 88de33ce06ab4c..1ec8f31264e2fe 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -157,6 +157,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,
diff --git a/test/parallel/test-worker-stack-overflow-stack-size.js b/test/parallel/test-worker-stack-overflow-stack-size.js
new file mode 100644
index 00000000000000..f218554297d1a3
--- /dev/null
+++ b/test/parallel/test-worker-stack-overflow-stack-size.js
@@ -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());