microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.51k stars 29.39k forks source link

Debugging node app pauses unexpectedly when a Worker is created #125451

Closed hogpilot closed 3 years ago

hogpilot commented 3 years ago

Issue Type: Bug

After updating my VS Code, my debugging experience became unpredictable and would pause unexpectedly. I've traced down the root cause in my code to an instantiation of a node Worker (from the worker_threads module).

I have created a simple reproducible scenario with instructions here.

The problem manifests when a worker is created with an env option that doesn't include NODE_OPTIONS in the environment variable. (NOTE: I wanted the worker thread to act as a sort of "safe" sandbox, with no knoweldge of the owning thread's environment settings):

  import { Worker } from 'worker_threads';

  // This line will cause an unexpected pause in the "Worker1" call stack 
  const worker = new Worker(new URL('./worker.js', import.meta.url), { env: {} } );

  // This line will not cause an unexpected pause in the call stack
  const worker = new Worker(new URL('./worker.js', import.meta.url), { env: {NODE_OPTIONS: process.env.NODE_OPTIONS} } );

Steps to Reproduce:

  1. Clone this repo: https://github.com/hogpilot/vscode-debug-test
  2. Open the repo in VS Code
  3. Run the following commands in the terminal:
    npm install
    npm run build
  4. From VS Code's "Run and Debug" tab, select "Launch Program" (from my launch.json)
  5. The code will pause in the Worker1 stack (without any breakpoints set) when it creates the new Worker

Workaround:

  1. When creating the new Worker with an env option, add NODE_OPTIONS to the env object.

NOTE: I am using webpack5 to build the code, following their guidance for instantiating worker threads

VS Code version: Code 1.56.2 (054a9295330880ed74ceaedda236253b4f39a335, 2021-05-12T17:13:13.157Z) OS version: Windows_NT x64 10.0.17763

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz (12 x 2592)| |GPU Status|2d_canvas: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: enabled
opengl: enabled_on
rasterization: enabled
skia_renderer: enabled_on
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled| |Load (avg)|undefined| |Memory (System)|31.73GB (11.64GB free)| |Process Argv|--crash-reporter-id f3ef9d22-974e-4c16-bdee-33c2342fccfa| |Screen Reader|no| |VM|0%|
Extensions (6) Extension|Author (truncated)|Version ---|---|--- vscode-eslint|dba|2.1.20 xml|Dot|2.5.1 vscode-env|Iro|0.1.0 mdx|sil|0.1.0 code-eol|soh|0.3.3 code-spell-checker|str|1.10.2
A/B Experiments ``` vsliv368:30146709 vsreu685:30147344 python383cf:30185419 pythonvspyt602:30300191 vspor879:30202332 vspor708:30202333 vspor363:30204092 vswsl492:30256859 pythonvspyt639:30300192 pythontb:30283811 pythonvspyt551:30311712 vspre833cf:30267465 pythonptprofiler:30281270 vshan820:30294714 vscoreces:30290705 pythondataviewer:30285071 vscus158cf:30286554 vscgsv2ct:30307505 vscorehov:30309549 bridgeflight:30312182 vscod805cf:30301675 ```
vscodebot[bot] commented 3 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

connor4312 commented 3 years ago

This appears to be an issue in Node.js, I've reported it here: https://github.com/nodejs/node/issues/38934

We could patch around it in js-debug, PR's welcome. Code pointer: https://github.com/microsoft/vscode-js-debug/blob/3a3436a299113045c69fe8e07bb1b70b6ecde950/src/adapter/threads.ts#L626

Patil2099 commented 3 years ago

I would love to help in solving this issue @connor4312. Can you guide me a little?

bl-ue commented 3 years ago

@Patil2099 take a look at https://github.com/nodejs/node/issues/38934 — maybe you could do some investigation there as to why it's behaving that way :)

connor4312 commented 3 years ago

Yea, you could patch Node (I'm not familiar with their internals enough to give pointers) or, inside _onPaused in js-debug, call this.resume() if we identify a paused worker. It looks like the event reason is "Break on start" in that case. You could make the type() in the NodeWorkerTarget something like node_worker, and then be able to identify in _onPaused that if the target is a node_worker and reason is break on start, then we should actually resume.