GeezerVison is showing github.com — src: skip inspector wait in internal workers · nodejs/node@91936eb
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: src/api/environment.cc +2-3Lines changed: 2 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -371,9 +371,8 @@ IsolateData* CreateIsolateData( | 371 | 371 | MultiIsolatePlatform* platform, | 372 | 372 | ArrayBufferAllocator* allocator, | 373 | 373 | const EmbedderSnapshotData* embedder_snapshot_data) { | 374 | | - const SnapshotData* snapshot_data = | 375 | | - SnapshotData::FromEmbedderWrapper(embedder_snapshot_data); | 376 | | - return new IsolateData(isolate, loop, platform, allocator, snapshot_data); | | 374 | + return IsolateData::CreateIsolateData( | | 375 | + isolate, loop, platform, allocator, embedder_snapshot_data); | 377 | 376 | } | 378 | 377 |
| 379 | 378 | void FreeIsolateData(IsolateData* isolate_data) { | |
Collapse file Copy file name to clipboardExpand all lines: src/env-inl.h -5Lines changed: 0 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -576,11 +576,6 @@ inline std::shared_ptr<PerIsolateOptions> IsolateData::options() { | 576 | 576 | return options_; | 577 | 577 | } | 578 | 578 |
| 579 | | -inline void IsolateData::set_options( | 580 | | - std::shared_ptr<PerIsolateOptions> options) { | 581 | | - options_ = std::move(options); | 582 | | -} | 583 | | - | 584 | 579 | template <typename Fn> | 585 | 580 | void Environment::SetImmediate(Fn&& cb, CallbackFlags::Flags flags) { | 586 | 581 | auto callback = native_immediates_.CreateCallback(std::move(cb), flags); | |
Collapse file Copy file name to clipboardExpand all lines: src/env.cc +20-4Lines changed: 20 additions & 4 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -535,19 +535,35 @@ Mutex IsolateData::isolate_data_mutex_; | 535 | 535 | std::unordered_map<uint16_t, std::unique_ptr<PerIsolateWrapperData>> | 536 | 536 | IsolateData::wrapper_data_map_; | 537 | 537 |
| | 538 | +IsolateData* IsolateData::CreateIsolateData( | | 539 | + Isolate* isolate, | | 540 | + uv_loop_t* loop, | | 541 | + MultiIsolatePlatform* platform, | | 542 | + ArrayBufferAllocator* allocator, | | 543 | + const EmbedderSnapshotData* embedder_snapshot_data, | | 544 | + std::shared_ptr<PerIsolateOptions> options) { | | 545 | + const SnapshotData* snapshot_data = | | 546 | + SnapshotData::FromEmbedderWrapper(embedder_snapshot_data); | | 547 | + if (options == nullptr) { | | 548 | + options = per_process::cli_options->per_isolate->Clone(); | | 549 | + } | | 550 | + return new IsolateData( | | 551 | + isolate, loop, platform, allocator, snapshot_data, options); | | 552 | +} | | 553 | + | 538 | 554 | IsolateData::IsolateData(Isolate* isolate, | 539 | 555 | uv_loop_t* event_loop, | 540 | 556 | MultiIsolatePlatform* platform, | 541 | 557 | ArrayBufferAllocator* node_allocator, | 542 | | - const SnapshotData* snapshot_data) | | 558 | + const SnapshotData* snapshot_data, | | 559 | + std::shared_ptr<PerIsolateOptions> options) | 543 | 560 | : isolate_(isolate), | 544 | 561 | event_loop_(event_loop), | 545 | 562 | node_allocator_(node_allocator == nullptr ? nullptr | 546 | 563 | : node_allocator->GetImpl()), | 547 | 564 | platform_(platform), | 548 | | - snapshot_data_(snapshot_data) { | 549 | | - options_.reset( | 550 | | - new PerIsolateOptions(*(per_process::cli_options->per_isolate))); | | 565 | + snapshot_data_(snapshot_data), | | 566 | + options_(std::move(options)) { | 551 | 567 | v8::CppHeap* cpp_heap = isolate->GetCppHeap(); | 552 | 568 |
| 553 | 569 | uint16_t cppgc_id = kDefaultCppGCEmbedderID; | |
Collapse file Copy file name to clipboardExpand all lines: src/env.h +14-5Lines changed: 14 additions & 5 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -138,12 +138,22 @@ struct PerIsolateWrapperData { | 138 | 138 | }; | 139 | 139 |
| 140 | 140 | class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { | 141 | | - public: | | 141 | + private: | 142 | 142 | IsolateData(v8::Isolate* isolate, | 143 | 143 | uv_loop_t* event_loop, | 144 | | - MultiIsolatePlatform* platform = nullptr, | 145 | | - ArrayBufferAllocator* node_allocator = nullptr, | 146 | | - const SnapshotData* snapshot_data = nullptr); | | 144 | + MultiIsolatePlatform* platform, | | 145 | + ArrayBufferAllocator* node_allocator, | | 146 | + const SnapshotData* snapshot_data, | | 147 | + std::shared_ptr<PerIsolateOptions> options); | | 148 | + | | 149 | + public: | | 150 | + static IsolateData* CreateIsolateData( | | 151 | + v8::Isolate* isolate, | | 152 | + uv_loop_t* event_loop, | | 153 | + MultiIsolatePlatform* platform = nullptr, | | 154 | + ArrayBufferAllocator* node_allocator = nullptr, | | 155 | + const EmbedderSnapshotData* embedder_snapshot_data = nullptr, | | 156 | + std::shared_ptr<PerIsolateOptions> options = nullptr); | 147 | 157 | ~IsolateData(); | 148 | 158 |
| 149 | 159 | SET_MEMORY_INFO_NAME(IsolateData) | @@ -172,7 +182,6 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { | 172 | 182 | inline MultiIsolatePlatform* platform() const; | 173 | 183 | inline const SnapshotData* snapshot_data() const; | 174 | 184 | inline std::shared_ptr<PerIsolateOptions> options(); | 175 | | - inline void set_options(std::shared_ptr<PerIsolateOptions> options); | 176 | 185 |
| 177 | 186 | inline NodeArrayBufferAllocator* node_allocator() const; | 178 | 187 |
| |
Collapse file Copy file name to clipboardExpand all lines: src/node_options-inl.h +7Lines changed: 7 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -17,6 +17,13 @@ EnvironmentOptions* PerIsolateOptions::get_per_env_options() { | 17 | 17 | return per_env.get(); | 18 | 18 | } | 19 | 19 |
| | 20 | +std::shared_ptr<PerIsolateOptions> PerIsolateOptions::Clone() const { | | 21 | + auto options = | | 22 | + std::shared_ptr<PerIsolateOptions>(new PerIsolateOptions(*this)); | | 23 | + options->per_env = std::make_shared<EnvironmentOptions>(*per_env); | | 24 | + return options; | | 25 | +} | | 26 | + | 20 | 27 | namespace options_parser { | 21 | 28 |
| 22 | 29 | template <typename Options> | |
Collapse file Copy file name to clipboardExpand all lines: src/node_options.h +13Lines changed: 13 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -94,6 +94,11 @@ class DebugOptions : public Options { | 94 | 94 | break_first_line = true; | 95 | 95 | } | 96 | 96 |
| | 97 | + void DisableWaitOrBreakFirstLine() { | | 98 | + inspect_wait = false; | | 99 | + break_first_line = false; | | 100 | + } | | 101 | + | 97 | 102 | bool wait_for_connect() const { | 98 | 103 | return break_first_line || break_node_first_line || inspect_wait; | 99 | 104 | } | @@ -245,6 +250,9 @@ class EnvironmentOptions : public Options { | 245 | 250 |
| 246 | 251 | class PerIsolateOptions : public Options { | 247 | 252 | public: | | 253 | + PerIsolateOptions() = default; | | 254 | + PerIsolateOptions(PerIsolateOptions&&) = default; | | 255 | + | 248 | 256 | std::shared_ptr<EnvironmentOptions> per_env { new EnvironmentOptions() }; | 249 | 257 | bool track_heap_objects = false; | 250 | 258 | bool report_uncaught_exception = false; | @@ -256,6 +264,11 @@ class PerIsolateOptions : public Options { | 256 | 264 | inline EnvironmentOptions* get_per_env_options(); | 257 | 265 | void CheckOptions(std::vector<std::string>* errors, | 258 | 266 | std::vector<std::string>* argv) override; | | 267 | + | | 268 | + inline std::shared_ptr<PerIsolateOptions> Clone() const; | | 269 | + | | 270 | + private: | | 271 | + PerIsolateOptions(const PerIsolateOptions&) = default; | 259 | 272 | }; | 260 | 273 |
| 261 | 274 | class PerProcessOptions : public Options { | |
Collapse file Copy file name to clipboardExpand all lines: src/node_worker.cc +21-11Lines changed: 21 additions & 11 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -185,16 +185,15 @@ class WorkerThreadData { | 185 | 185 | isolate->SetStackLimit(w->stack_base_); | 186 | 186 |
| 187 | 187 | HandleScope handle_scope(isolate); | 188 | | - isolate_data_.reset( | 189 | | - CreateIsolateData(isolate, | 190 | | - &loop_, | 191 | | - w_->platform_, | 192 | | - allocator.get(), | 193 | | - w->snapshot_data()->AsEmbedderWrapper().get())); | | 188 | + isolate_data_.reset(IsolateData::CreateIsolateData( | | 189 | + isolate, | | 190 | + &loop_, | | 191 | + w_->platform_, | | 192 | + allocator.get(), | | 193 | + w->snapshot_data()->AsEmbedderWrapper().get(), | | 194 | + std::move(w_->per_isolate_opts_))); | 194 | 195 | CHECK(isolate_data_); | 195 | 196 | CHECK(!isolate_data_->is_building_snapshot()); | 196 | | - if (w_->per_isolate_opts_) | 197 | | - isolate_data_->set_options(std::move(w_->per_isolate_opts_)); | 198 | 197 | isolate_data_->set_worker_context(w_); | 199 | 198 | isolate_data_->max_young_gen_size = | 200 | 199 | params.constraints.max_young_generation_size_in_bytes(); | @@ -491,9 +490,8 @@ Worker::~Worker() { | 491 | 490 |
| 492 | 491 | void Worker::New(const FunctionCallbackInfo<Value>& args) { | 493 | 492 | Environment* env = Environment::GetCurrent(args); | 494 | | - auto is_internal = args[5]; | 495 | | - CHECK(is_internal->IsBoolean()); | 496 | | - if (is_internal->IsFalse()) { | | 493 | + bool is_internal = args[5]->IsTrue(); | | 494 | + if (!is_internal) { | 497 | 495 | THROW_IF_INSUFFICIENT_PERMISSIONS( | 498 | 496 | env, permission::PermissionScope::kWorkerThreads, ""); | 499 | 497 | } | @@ -658,7 +656,19 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) { | 658 | 656 | return; | 659 | 657 | } | 660 | 658 | } else { | | 659 | + // Copy the parent's execArgv. | 661 | 660 | exec_argv_out = env->exec_argv(); | | 661 | + per_isolate_opts = env->isolate_data()->options()->Clone(); | | 662 | + } | | 663 | + | | 664 | + // Internal workers should not wait for inspector frontend to connect or | | 665 | + // break on the first line of internal scripts. Module loader threads are | | 666 | + // essential to load user codes and must not be blocked by the inspector | | 667 | + // for internal scripts. | | 668 | + // Still, `--inspect-node` can break on the first line of internal scripts. | | 669 | + if (is_internal) { | | 670 | + per_isolate_opts->per_env->get_debug_options() | | 671 | + ->DisableWaitOrBreakFirstLine(); | 662 | 672 | } | 663 | 673 |
| 664 | 674 | const SnapshotData* snapshot_data = env->isolate_data()->snapshot_data(); | |
Collapse file Copy file name to clipboardExpand all lines: test/common/inspector-helper.js +4Lines changed: 4 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -309,6 +309,10 @@ class InspectorSession { | 309 | 309 | return notification.method === 'Runtime.executionContextDestroyed' && | 310 | 310 | notification.params.executionContextId === 1; | 311 | 311 | }); | | 312 | + await this.waitForDisconnect(); | | 313 | + } | | 314 | + | | 315 | + async waitForDisconnect() { | 312 | 316 | while ((await this._instance.nextStderrString()) !== | 313 | 317 | 'Waiting for the debugger to disconnect...'); | 314 | 318 | await this.disconnect(); | |
Collapse file Copy file name to clipboard +34Lines changed: 34 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// This tests esm loader's internal worker will not be blocked by --inspect-brk. | | 2 | +// Regression: https://github.com/nodejs/node/issues/53681 | | 3 | + | | 4 | +'use strict'; | | 5 | +const common = require('../common'); | | 6 | + | | 7 | +common.skipIfInspectorDisabled(); | | 8 | + | | 9 | +const assert = require('assert'); | | 10 | +const fixtures = require('../common/fixtures'); | | 11 | +const { NodeInstance } = require('../common/inspector-helper.js'); | | 12 | + | | 13 | +async function runIfWaitingForDebugger(session) { | | 14 | + const commands = [ | | 15 | + { 'method': 'Runtime.enable' }, | | 16 | + { 'method': 'Debugger.enable' }, | | 17 | + { 'method': 'Runtime.runIfWaitingForDebugger' }, | | 18 | + ]; | | 19 | + | | 20 | + await session.send(commands); | | 21 | + await session.waitForNotification('Debugger.paused'); | | 22 | +} | | 23 | + | | 24 | +async function runTest() { | | 25 | + const main = fixtures.path('es-module-loaders', 'register-loader.mjs'); | | 26 | + const child = new NodeInstance(['--inspect-brk=0'], '', main); | | 27 | + | | 28 | + const session = await child.connectInspectorSession(); | | 29 | + await runIfWaitingForDebugger(session); | | 30 | + await session.runToCompletion(); | | 31 | + assert.strictEqual((await child.expectShutdown()).exitCode, 0); | | 32 | +} | | 33 | + | | 34 | +runTest(); |
Collapse file Copy file name to clipboard +33Lines changed: 33 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 1 | +// This tests esm loader's internal worker will not be blocked by --inspect-wait. | | 2 | +// Regression: https://github.com/nodejs/node/issues/53681 | | 3 | + | | 4 | +'use strict'; | | 5 | +const common = require('../common'); | | 6 | + | | 7 | +common.skipIfInspectorDisabled(); | | 8 | + | | 9 | +const assert = require('assert'); | | 10 | +const fixtures = require('../common/fixtures'); | | 11 | +const { NodeInstance } = require('../common/inspector-helper.js'); | | 12 | + | | 13 | +async function runIfWaitingForDebugger(session) { | | 14 | + const commands = [ | | 15 | + { 'method': 'Runtime.enable' }, | | 16 | + { 'method': 'Debugger.enable' }, | | 17 | + { 'method': 'Runtime.runIfWaitingForDebugger' }, | | 18 | + ]; | | 19 | + | | 20 | + await session.send(commands); | | 21 | +} | | 22 | + | | 23 | +async function runTest() { | | 24 | + const main = fixtures.path('es-module-loaders', 'register-loader.mjs'); | | 25 | + const child = new NodeInstance(['--inspect-wait=0'], '', main); | | 26 | + | | 27 | + const session = await child.connectInspectorSession(); | | 28 | + await runIfWaitingForDebugger(session); | | 29 | + await session.waitForDisconnect(); | | 30 | + assert.strictEqual((await child.expectShutdown()).exitCode, 0); | | 31 | +} | | 32 | + | | 33 | +runTest(); |
|