src: fix Stop() terminating the next Environment on the isolate · nodejs/node@afc3e55 · GitHub
Skip to content

Commit afc3e55

Browse files
codebytereaduh95
authored andcommitted
src: fix Stop() terminating the next Environment on the isolate
After `Stop(env)`, freeing the Environment and creating another one on the same isolate failed whenever no JavaScript ran in between: the new Environment's first script was terminated before it started. That is the normal case when `Stop()` is called from the process exit handler for an uncaught exception, or by an embedder while the loop is idle. `Stop()` calls `isolate->TerminateExecution()` unless `kDoNotTerminateIsolate` is set, and V8 only clears that request the next time JavaScript runs, so it outlived the Environment it was meant for. Cancel a pending termination when the Environment it was meant for is freed. `Worker::Run()` already did this by hand before freeing its Environment, with a TODO asking why V8 hit a DCHECK without it; this is why, and that call now happens in `FreeEnvironment()`. Refs: #33347 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65819 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 858702e commit afc3e55

3 files changed

Lines changed: 24 additions & 5 deletions

File tree

‎src/api/environment.cc‎

Lines changed: 3 additions & 0 deletions

‎src/node_worker.cc‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,6 @@ void Worker::Run() {
326326

327327
DeleteFnPtr<Environment, FreeEnvironment> env_;
328328
auto cleanup_env = OnScopeLeave([&]() {
329-
// TODO(addaleax): This call is harmless but should not be necessary.
330-
// Figure out why V8 is raising a DCHECK() here without it
331-
// (in test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js).
332-
isolate_->CancelTerminateExecution();
333-
334329
if (!env_) return;
335330
env_->set_can_call_into_js(false);
336331

‎test/cctest/test_environment.cc‎

Lines changed: 21 additions & 0 deletions

0 commit comments

Comments
 (0)