debugger: propagate --debug-port= to debuggee

[bnoordhuis]

Before this commit node --debug-port=1234 debug t.js ignored the
--debug-port= argument, binding to the default port 5858 instead,
making it impossible to debug more than one process on the same
machine that way.

This commit also reduces the number of places where the default port
is hard-coded by one.

Fixes: #3345

R=@indutny

CI: https://ci.nodejs.org/job/node-test-pull-request/550/

[indutny]

LGTM

[bnoordhuis]

One more CI with a fix-up for Windows path munging in the test: https://ci.nodejs.org/job/node-test-pull-request/551/

[jasnell]

LGTM

[bnoordhuis]

It looks like the test is hitting a race condition in the debugger where the child processes get stuck waiting on a semaphore...

(gdb) thread apply 1 bt 10 Thread 1 (Thread 0x7f92e89e6740 (LWP 17569)): #0 0x00007f92e792a829 in do_futex_wait.constprop () from /lib64/libpthread.so.0 #1 0x00007f92e792a8c4 in __new_sem_wait_slow.constprop.0 () from /lib64/libpthread.so.0 #2 0x00007f92e792a96a in sem_wait@@GLIBC_2.2.5 () from /lib64/libpthread.so.0 #3 0x0000000000e11728 in v8::base::Semaphore::Wait() () #4 0x00000000009b3ad4 in v8::internal::Debug::NotifyMessageHandler(v8::DebugEvent, v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::JSObject>, bool) () #5 0x00000000009b44c8 in v8::internal::Debug::ProcessDebugEvent(v8::DebugEvent, v8::internal::Handle<v8::internal::JSObject>, bool) () #6 0x00000000009b45c9 in v8::internal::Debug::OnDebugBreak(v8::internal::Handle<v8::internal::Object>, bool) () #7 0x00000000009b616b in v8::internal::Debug::Break(v8::internal::Arguments, v8::internal::JavaScriptFrame*) () #8 0x0000000000c01be3 in v8::internal::Runtime_DebugBreak(int, v8::internal::Object**, v8::internal::Isolate*) () #9 0x000010de5e0060bb in ?? () (More stack frames follow...) (gdb) thread apply 2 bt 10 Thread 2 (Thread 0x7f92e5557700 (LWP 17589)): #0 0x00007f92e7658eb9 in syscall () from /lib64/libc.so.6 #1 0x0000000000e0d18a in uv__epoll_wait (epfd=<optimized out>, events=events@entry=0x7f92e5553d50, nevents=nevents@entry=1024, timeout=timeout@entry=-1) at ../deps/uv/src/unix/linux-syscalls.c:321 #2 0x0000000000e0b04e in uv__io_poll (loop=loop@entry=0x26c85e0, timeout=-1) at ../deps/uv/src/unix/linux-core.c:243 #3 0x0000000000dfc5a2 in uv_run (loop=0x26c85e0, mode=UV_RUN_DEFAULT) at ../deps/uv/src/unix/core.c:341 #4 0x0000000000d648c1 in node::debugger::Agent::WorkerRun() () #5 0x0000000000e07ac8 in uv__thread_start (arg=<optimized out>) at ../deps/uv/src/unix/thread.c:49 #6 0x00007f92e7923555 in start_thread () from /lib64/libpthread.so.0 #7 0x00007f92e765eb9d in clone () from /lib64/libc.so.6 

Happens locally about once every ~50 runs. Will investigate.

EDIT: Interestingly enough I've also seen cases where the main thread is already gone but the debugger thread is still around, sleeping in epoll_wait.

EDIT2: It may be much more trivial than that. Looks like killing the debugger doesn't always terminate the debuggee.

[MylesBorins]

@bnoordhuis what ended up happening with this?

[bnoordhuis]

@thealphanerd It's basically blocked by #5368 - v8::Debug::DebugBreak() is no longer async signal-safe. I'm going to revisit this PR once that issue is fixed - if it gets fixed.

[Trott]

@bnoordhuis#5368 was closed by #5904. Does that mean this is ready to move forward again? Or is life not that simple?

[bnoordhuis]

This should be good to go again. I actually had "dusting off debugger PRs" on my TODO list for this week.

Rebased + new CI: https://ci.nodejs.org/job/node-test-pull-request/2778/

[cjihrig]

LGTM. There were some CI failures that all look unrelated, but the number of failures might warrant another CI.

[Trott]

The (hopefully unrelated) failures are all EADDRINUSE on localhost:common.PORT. Since this adds a test that uses that address/port, it's Not Impossible that it doesn't let that port go from time to time and causes the other tests to fail. I kind of doubt it, but since debugger tests are pretty much the most unpredictable thing that has ever existed in terms of side effects, let's try again: https://ci.nodejs.org/job/node-test-pull-request/2786/

[bnoordhuis]

Le sigh, Java exceptions on the ARM bots. Third time is the charm: https://ci.nodejs.org/job/node-test-pull-request/2788/

[Trott]

Probably fine to table-flip the CI and land the code at this point but hey, I loves me some CI runs, so here's another one: https://ci.nodejs.org/job/node-test-pull-request/2790/

[bnoordhuis]

This time it was the ARM bots and one of the plinux machines... =)

I'm confident enough it works though. Landed in 18fb4f9 and added the lts-watch-v4.x tag. Thanks for the reviews, everyone.

[Trott]

The (hopefully unrelated) failures are all EADDRINUSE on localhost:common.PORT. Since this adds a test that uses that address/port, it's Not Impossible that it doesn't let that port go from time to time and causes the other tests to fail.

It looks like that may be exactly what's going on and why (or at least one reason why) the last couple days have been unpleasant in CI-land...

https://ci.nodejs.org/job/node-stress-single-test/nodes=freebsd102-64/748/console

+ OK=5 + echo '5 OK: 5 NOT OK: 0 TOTAL: 999' 5 OK: 5 NOT OK: 0 TOTAL: 999 + for i in '`seq $RUN_TIMES`' + python tools/test.py -p tap --mode=release parallel/test-debug-port-numbers 1..1 ok 1 parallel/test-debug-port-numbers --- duration_ms: 1.376 ... + OK=6 + echo '6 OK: 6 NOT OK: 0 TOTAL: 999' 6 OK: 6 NOT OK: 0 TOTAL: 999 + for i in '`seq $RUN_TIMES`' + python tools/test.py -p tap --mode=release parallel/test-debug-port-numbers 1..1 not ok 1 parallel/test-debug-port-numbers # TIMEOUT # debug> debug> debug> debug> �< Debugger listening on port 12346�< Debugger listening on port 12347 # debug> �connecting to 127.0.0.1:12347 ... # ��debug> < Debugger listening on port 12348< Error: listen EADDRINUSE :::12349�connecting to 127.0.0.1:12346 ... # debug> �connecting to 127.0.0.1:12348 .. # debug> �connecting to 127.0.0.1:12349 .... ok # debug> ok # debug> ok # debug> �< at Object.exports._errnoException (util.js:1007:11) # < at exports._exceptionWithHostPort (util.js:1030:20) # < at Agent.Server._listen2 (net.js:1253:14) # < at listen (net.js:1289:10) # < at Agent.Server.listen (net.js:1385:5) # < at Object.start (_debug_agent.js:21:9) # < at startup (node.js:74:44) # < at node.js:444:3 # debug> ok # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> --- duration_ms: 60.65 ... + NOK=1 + echo '7 OK: 6 NOT OK: 1 TOTAL: 999' 7 OK: 6 NOT OK: 1 TOTAL: 999 + for i in '`seq $RUN_TIMES`' + python tools/test.py -p tap --mode=release parallel/test-debug-port-numbers 1..1 not ok 1 parallel/test-debug-port-numbers # TIMEOUT # debug> debug> debug> debug> ��< Error: listen EADDRINUSE :::12349 # < Debugger listening on port 12347debug> �connecting to 127.0.0.1:12349 .. # .�< Debugger listening on port 12346 # debug> �connecting to 127.0.0.1:12347 ...��< Debugger listening on port 12348< at Object.exports._errnoException (util.js:1007:11) # < at exports._exceptionWithHostPort (util.js:1030:20) # < at Agent.Server._listen2 (net.js:1253:14) # < at listen (net.js:1289:10) # < at Agent.Server.listen (net.js:1385:5) # < at Object.start (_debug_agent.js:21:9) # < at startup (node.js:74:44) # < at node.js:444:3 # # debug> debug> �connecting to 127.0.0.1:12348 ..debug> ok # . ok # debug> debug> �connecting to 127.0.0.1:12346 ... ok # debug> ok # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> �break in test/parallel/test-debug-port-numbers.js:1 # �> 1 'use strict'; # � 2 # � 3 const common = require('../common'); # debug> 

[bnoordhuis]

Continues in #7034.