Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

An error occurred while running vm with breakOnSignt option in multiple worker_threads #43699

Closed
luckyyyyy opened this issue Jul 6, 2022 · 1 comment · Fixed by #43780
Closed
Labels
vm Issues and PRs related to the vm subsystem. worker Issues and PRs related to Worker support.

Comments

@luckyyyyy
Copy link

luckyyyyy commented Jul 6, 2022

Version

12 / 14 / 16 / 18 and current LTS

Platform

all

Subsystem

vm

What steps will reproduce the bug?

const { Worker, isMainThread } = require('worker_threads');
const vm = require('vm')

if (isMainThread) {
    for (let i = 0; i < 2; i++) {
        new Worker(__filename);
    }
} else {
    const ctx = vm.createContext({});
    setInterval(() => {
        vm.runInContext('console.log(1)', ctx, { breakOnSigint: true })
    }, 0)
}

How often does it reproduce? Is there a required condition?

Always

What do you see instead?

node/out/Debug/node[10185]: ../../src/node_watchdog.cc:387:void node::SigintWatchdogHelper::Unregister(node::SigintWatchdogBase *): Assertion `(it) != (watchdogs_.end())' failed.
 1: 0x104d4d3c0 node::DumpBacktrace(__sFILE*) [/Users/william/workspace/node/out/Debug/node]
 2: 0x104e9fcd0 node::Abort() [/Users/william/workspace/node/out/Debug/node]
 3: 0x104e9f8cc node::AppendExceptionLine(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Message>, node::ErrorHandlingMode) [/Users/william/workspace/node/out/Debug/node]
 4: 0x105065c2c node::SigintWatchdogHelper::Unregister(node::SigintWatchdogBase*) [/Users/william/workspace/node/out/Debug/node]
 5: 0x105065b70 node::SigintWatchdog::~SigintWatchdog() [/Users/william/workspace/node/out/Debug/node]
 6: 0x105065e2c node::SigintWatchdog::~SigintWatchdog() [/Users/william/workspace/node/out/Debug/node]
 7: 0x104e731e0 node::contextify::ContextifyScript::EvalMachine(node::Environment*, long long, bool, bool, bool, std::__1::shared_ptr<v8::MicrotaskQueue>, v8::FunctionCallbackInfo<v8::Value> const&) [/Users/william/workspace/node/out/Debug/node]
 8: 0x104e72458 node::contextify::ContextifyScript::RunInContext(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/william/workspace/node/out/Debug/node]
 9: 0x1053ae410 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/william/workspace/node/out/Debug/node]
10: 0x1053accc8 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/william/workspace/node/out/Debug/node]
11: 0x1053ab40c v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/Users/william/workspace/node/out/Debug/node]
12: 0x105fec9cc Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/william/workspace/node/out/Debug/node]
13: 0x150057eb4 
14: 0x15005a8c8 
15: 0x150056a8c 
16: 0x1500597b8 
17: 0x15005790c 
18: 0x105f6feac Builtins_JSEntryTrampoline [/Users/william/workspace/node/out/Debug/node]
19: 0x105f6fb44 Builtins_JSEntry [/Users/william/workspace/node/out/Debug/node]
20: 0x105519570 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/william/workspace/node/out/Debug/node]
21: 0x10551877c v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) [/Users/william/workspace/node/out/Debug/node]
22: 0x105300ffc v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [/Users/william/workspace/node/out/Debug/node]
23: 0x104d7f1b8 node::Environment::RunTimers(uv_timer_s*) [/Users/william/workspace/node/out/Debug/node]
24: 0x105f3c540 uv__run_timers [/Users/william/workspace/node/out/Debug/node]
25: 0x105f43548 uv_run [/Users/william/workspace/node/out/Debug/node]
26: 0x104cd6b78 node::SpinEventLoop(node::Environment*) [/Users/william/workspace/node/out/Debug/node]
27: 0x10506b2b4 node::worker::Worker::Run() [/Users/william/workspace/node/out/Debug/node]
28: 0x105072338 node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_3::operator()(void*) const [/Users/william/workspace/node/out/Debug/node]
29: 0x1050722e4 node::worker::Worker::StartThread(v8::FunctionCallbackInfo<v8::Value> const&)::$_3::__invoke(void*) [/Users/william/workspace/node/out/Debug/node]
30: 0x1c0a8826c _pthread_start [/usr/lib/system/libsystem_pthread.dylib]
31: 0x1c0a8308c thread_start [/usr/lib/system/libsystem_pthread.dylib]
[1]    10185 abort      node/out/Debug/node index.js
@VoltrexKeyva VoltrexKeyva added vm Issues and PRs related to the vm subsystem. worker Issues and PRs related to Worker support. labels Jul 6, 2022
@theanarkh
Copy link
Contributor

theanarkh commented Jul 6, 2022

I think because modifying the watchdogs_ and start_stop_count_ of SigintWatchdogHelper is not a atomic operation. And the related code is as follows.

SigintWatchdog::SigintWatchdog(v8::Isolate* isolate, bool* received_signal) : isolate_(isolate), received_signal_(received_signal) {
  SigintWatchdogHelper::GetInstance()->Register(this);
  SigintWatchdogHelper::GetInstance()->Start();
}


SigintWatchdog::~SigintWatchdog() {
  SigintWatchdogHelper::GetInstance()->Unregister(this);
  SigintWatchdogHelper::GetInstance()->Stop();
}

The operation as follows will trigger the bug:

  1. worker1 call Register and Start: watchdogs_,size() is 1 and start_stop_count_ is 1.
  2. worker2 call Register: watchdogs_,size() is 2 and start_stop_count_ is 1.
  3. worker1 call UnRegister and Stop: because start_stop_count_ is 1, So SigintWatchdogHelper will clear watchdogs_. So watchdogs_.size() is 0 and start_stop_count_ is 0.
  4. worker1 call Start: watchdogs_.size() is 0 and start_stop_count_ is 1.
  5. worker1 call UnRegister: because watchdogs_.size() is 0 so CHECK_NE(it, watchdogs_.end()); is failed.

I modify the code to verify this.

Mutex mutex_;

SigintWatchdog::SigintWatchdog(
  v8::Isolate* isolate, bool* received_signal)
    : isolate_(isolate), received_signal_(received_signal) {
  Mutex::ScopedLock lock(mutex_);
  SigintWatchdogHelper::GetInstance()->Register(this);
  SigintWatchdogHelper::GetInstance()->Start();
}

SigintWatchdog::~SigintWatchdog() {
  Mutex::ScopedLock lock(mutex_);
  SigintWatchdogHelper::GetInstance()->Unregister(this);
  SigintWatchdogHelper::GetInstance()->Stop();
}

It works.

nodejs-github-bot pushed a commit that referenced this issue Jul 14, 2022
PR-URL: #43780
Fixes: #43699
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
danielleadams pushed a commit that referenced this issue Jul 26, 2022
PR-URL: #43780
Fixes: #43699
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
PR-URL: #43780
Fixes: #43699
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
PR-URL: #43780
Fixes: #43699
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
PR-URL: nodejs/node#43780
Fixes: nodejs/node#43699
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Issues and PRs related to the vm subsystem. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants