src: fix cb scope bugs involved in termination
An inheritor of https://github.com/nodejs/node/pull/45422 and a fix to https://github.com/nodejs/node/issues/43084. I move the original failing test back to the test/parallel/ to increase the chance that it exercises the defect, at which this PR is targeted.
To see the bug in 43084, apply this patch and run the script
diff --git a/src/env.cc b/src/env.cc
index 2f0720d402..5a4d9b2146 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -27,6 +27,8 @@
#include <iostream>
#include <limits>
#include <memory>
+#include <thread>
+#include <chrono>
namespace node {
@@ -907,6 +909,7 @@ void Environment::InitializeLibuv() {
void Environment::ExitEnv() {
set_can_call_into_js(false);
set_stopping(true);
+ std::this_thread::sleep_for(std::chrono::milliseconds(2000));
isolate_->TerminateExecution();
SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
}
// Flags: --expose-internals
'use strict';
const common = require('../common');
const { Worker } = require('worker_threads');
/**
* This file is a minimal possible reproduction and an
* explanation of https://github.com/nodejs/node/issues/43084.
*
* It can not trigger the bug consistently because
* that would require a rare thread execution interleaving
* condition.
*
* test-worker-http2-stream-terminate.js has a bigger chance
* to trigger the bug.
*/
if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = 1;
const workerData = new Int32Array(new SharedArrayBuffer(4));
const w = new Worker(__filename, { workerData });
w.on('exit', common.mustCall());
// Wait for the worker to enter the _write() below
// eslint-disable-next-line
while (Atomics.load(workerData, 0) !== 1) {}
w.terminate(); // node::Environment::ExitEnv() is called under the hood
return;
}
// --------- worker thread ---------
const JSStreamSocket = require('internal/js_stream_socket');
const { Duplex } = require('stream');
class DummyDuplex extends Duplex {
_read() {
}
_write(chunk, encoding, callback) {
/**
* Now, this worker thread has a stack looks like:
* _pthread_start
* ...
* node::CheckImmediate
* ...
* node::MakeCallback
* ...
* v8::internal::FunctionCallbackArguments::Call
* ...
* node::MakeCallback
* ...
*/
const { workerData } = require('worker_threads');
Atomics.store(workerData, 0, 1);
// Make some time for the main thread to enter node::Environment::ExitEnv()
// eslint-disable-next-line
for (let i = 0; i < 1000000; i++) {}
// Just before this function returns, if the main thread
// 1. called node::Environment::ExitEnv()/set_can_call_into_js(false)
// 2. but did not called node::Environment::ExitEnv()/TerminateExecution() yet
// 3. and got suspended by the OS
// Without the fix, this bug https://github.com/nodejs/node/issues/43084 occurs
// and emitAfter() in processImmediate() in lib/internal/timers.js complains
// Error: async hook stack has become corrupted (actual: 6, expected: 5)
}
_final(callback) {
}
}
setImmediate(() => {
const sock = new JSStreamSocket(new DummyDuplex());
sock.write(Buffer.alloc(10));
// JSStreamSocket basically delegate r/w to the Duplex paased to its constructor
// It is a net.Socket
// lib/net.js write()
// lib/internal/stream_base_commons.js writeGeneric()
// handleWriteReq()
// handle.writeBuffer()
// node::StreamBase::WriteBuffer()
// node::StreamBase::Write()
// node::JSStream::DoWrite()
// call to the above DummyDuplex
});