nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.34k stars 29.47k forks source link

Inspector does not initialize when using CreateEnvironment #31258

Open jc-lab opened 4 years ago

jc-lab commented 4 years ago

Inspector does not initialize when using CreateEnvironment. So occur crash in the code below. (inspectoragent()->client is null)

https://github.com/nodejs/node/blob/1a552f614b4ec2b9faf0bf932ad4e6ef4fdcaf01/src/node_worker.cc#L79-L80 https://github.com/nodejs/node/blob/3e79c004fdb93d01618fd90f0df934ac12c62353/src/inspector_agent.cc#L1010

Initialization takes place only here.

https://github.com/nodejs/node/blob/1a552f614b4ec2b9faf0bf932ad4e6ef4fdcaf01/src/node_main_instance.cc#L225-L231

Environment* CreateEnvironment(IsolateData* isolate_data,
                               Local<Context> context,
                               int argc,
                               const char* const* argv,
                               int exec_argc,
                               const char* const* exec_argv) {
  Isolate* isolate = context->GetIsolate();
  HandleScope handle_scope(isolate);
  Context::Scope context_scope(context);
  // TODO(addaleax): This is a much better place for parsing per-Environment
  // options than the global parse call.
  std::vector<std::string> args(argv, argv + argc);
  std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
  // TODO(addaleax): Provide more sensible flags, in an embedder-accessible way.
  Environment* env = new Environment(
      isolate_data,
      context,
      args,
      exec_args,
      static_cast<Environment::Flags>(Environment::kIsMainThread |
                                      Environment::kOwnsProcessState |
                                      Environment::kOwnsInspector));
  env->InitializeLibuv(per_process::v8_is_profiling);
  // ========== START SUGGEST ==========
  env->InitializeDiagnostics();

  // TODO(joyeecheung): when we snapshot the bootstrapped context,
  // the inspector and diagnostics setup should after after deserialization.
#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
  //TODO(jc-lab): If InitializeInspector fails, the caller must be informed of the return_code.
  env->InitializeInspector(nullptr);
#endif

I thought about the above suggestions. However, the above proposal does not pass the EnvironmentTest.MultipleEnvironmentsPerIsolate test. https://github.com/nodejs/node/blob/1a552f614b4ec2b9faf0bf932ad4e6ef4fdcaf01/src/inspector_agent.cc#L770

addaleax commented 4 years ago

https://github.com/nodejs/node/pull/30467 should fix this but it’s been blocked for a while, it’s on the top of my TODO list but I haven’t quite gotten back to it yet.

jc-lab commented 4 years ago

It is maybe resolved by #32672. But I haven't tested it.