Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

No call to SetIdle() when terminating #45422

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ void InternalCallbackScope::Close() {
if (closed_) return;
closed_ = true;

if (!env_->can_call_into_js()) return;

Isolate* isolate = env_->isolate();
auto idle = OnScopeLeave([&]() { isolate->SetIdle(true); });

if (!env_->can_call_into_js()) return;
auto perform_stopping_check = [&]() {
if (env_->is_stopping()) {
MarkAsFailed();
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-unhandled-exception-with-worker-inuse.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';
require('../common');

// Check that node will not call v8::Isolate::SetIdle() when exiting
// due to an unhandled exception, otherwise the assertion(enabled in
// debug build only) in the SetIdle() will fail.
//
// The root cause of this issue is that before PerIsolateMessageListener()
// is invoked by v8, v8 preserves the vm state, which is JS. However,
// SetIdle() requires the vm state is either EXTERNEL or IDLE when embedder
// calling it.

if (process.argv[2] === 'child') {
const { Worker } = require('worker_threads');
new Worker('', { eval: true });
throw new Error('xxx');
} else {
const assert = require('assert');
const { spawnSync } = require('child_process');
const result = spawnSync(process.execPath, [__filename, 'child']);

const stderr = result.stderr.toString().trim();
// Expect error message to be preserved
assert.match(stderr, /xxx/);
// Expect no crash message
assert.doesNotMatch(stderr, /node::DumpBacktrace/);
}