microsoft / vscode-js-debug

A DAP-compatible JavaScript debugger. Used in VS Code, VS, + more
MIT License
1.67k stars 283 forks source link

Debugger may hang if a firewall is active #2004

Closed tusharsnx closed 5 months ago

tusharsnx commented 6 months ago

Describe the bug

https://github.com/microsoft/vscode-js-debug/blob/6e29d1de451b2949477d15bcdb126187a16fd93b/src/common/findOpenPortSync.ts#L32

The vscode js debugger seems to use nc (netcat) for testing arbitrary ports to be available for the debugger to use. This works fine, except when a firewall is active on the system which can block access to ports in a way where the application would never see a reply coming back so they would just hang. In this case, nc never receives a reply and keeps waiting for it , and the debugger blocks on that (until it eventually times out).

I'm using WSL's new networkingMode=mirrored mode that comes with a firewall that by default blocks requests in the way I described above (no reply sent). I think it's important to wrap the call with a reasonable timeout that is also small. nc does give an option to set a timeout with -w option. This should be set within a range of a few seconds.

To Reproduce Steps to reproduce the behavior:

  1. Add this to .wslconfig:
    [wsl2]
    networkingMode = mirrored
  2. Use the snippets below :
    
    // main.js

const output = require('node:child_process').execSync("npm config get registry"); console.log(output.toString());

```jsonc
// launch.json

{
    "version": "0.2.0",
    "configurations": [
        {
            "type": "node",
            "request": "attach",
            "name": "Attach Program",
        }
    ]
}
  1. Add a breakpoint to line 1 (execSync() line) in the program.
  2. Now run the program, and attach to it:
    NODE_OPTIONS="--inspect-brk" node ./main.js
  3. Once the debugger is attached, press "Step over" and observe the program being hung.

In the background, node has started npm config get registry, but before the npm's node instance could run, node called the debugger code, and the debugger called nc command that is now hung waiting for a reply. On my system, it eventually resumes after the nc returns due to the (~2 mins) default timeout (?)

VS Code: Version: 1.88.1 (user setup) OS: Windows_NT x64 10.0.22631

tusharsnx commented 6 months ago

If you happen to have the PID of the attached node instance, you can run this command to see nc being stuck:

$ pstree -c -a -T -s <PID>

image

And once nc returns, this is how it looks:

image

connor4312 commented 6 months ago

Thanks for the investigation, I think this might have been the cause of another WSL bug we got recently. I'll look into a fix.

connor4312 commented 6 months ago

This should be fixed in the next nightly build, please let me know whether this fixes it for your

tusharsnx commented 6 months ago

Hey, thanks for fixing this so fast!

I think the new change might still cause some issues. spawnSync('lsof', ['-i', `@127.0.0.1:${+port}`]) doesn't account for ports that are opened by applications running at a higher privilege. There will be some false positives when a port is opened by an application, and yet lsof wouldn't list them when running it without sudo. However, that is a separate issue so I'm fine.

FWIW, the testSlow() strategy that uses createServer() seems to work flawlessly.

eleanorjboyd commented 5 months ago

Hi @connor4312 - seems like there still exists an outstanding issue that @tusharsnx describes. Is another issue tracking this or should I reopen this issue? Thanks

TylerLeonhardt commented 5 months ago

reopening based on @tusharsnx's comment.

connor4312 commented 5 months ago

Let's track that as a separate issue. The fix is still good for the original bug and I'm not sure I want to change it -- taking the performance hit for the pretty rare case of a higher privilege process listening on a high number port that we randomly pick.