inspector: Use default uv_listen backlog size

[eugeneo]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[eugeneo]

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

So far, this issue was only hit by a test case but there's a potential for it to be hit if some tool sends too many requests too fast.

[Trott]

The provided test doesn't fail when node is compiled without the source change. Shouldn't it? Or is it a "sometimes it fails, sometimes it doesn't" kind of test?

[Trott]

The provided test doesn't fail when node is compiled without the source change. Shouldn't it? Or is it a "sometimes it fails, sometimes it doesn't" kind of test?

The problem is that if it starts failing that way, then no one is going to think it's a bug in the code. They're going to think the test is unreliable and "fix" it rather than the bug in the code.

[Trott]

Although I did just confirm that this un-breaks parallel/test-inspector-multisession-ws from #20137 on macOS. So, maybe that's the "real" test for this. My question above is non-blocking (at least, non-blocking for me).

[Trott]

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

[Trott]

@nodejs/v8-inspector

[eugeneo]

The provided test doesn't fail when node is compiled without the source change. Shouldn't it? Or is it a "sometimes it fails, sometimes it doesn't" kind of test?

It reliably fails for me on macOS. But it is timing related - new client needs to attempt connection while the inspector still have not accepted the previous connection.

[eugeneo]

Looks like the test fails on Docker bots. I will reduce the number of connection attempts to 10 and will try again. I tried 10 connections on my Mac and I see failures roughly 8 runs out of 10.

[ofrobots]

I am trying to find out how the value 0 ends up picking the uv default size. AFAICT, uv_listen simply passes this onto listen(2) here. I see no indication that 0 is special as far as uv is concerned.

In the man pages for either mac or on Linux, I cannot find reference to the value 0 being special either. Am I missing something?

[eugeneo]

@ofrobots you are right, that seems to be a bad value. I simply tried it while debugging the test value, it worked on Mac but seems to be a bad value for Linux. I changed to 511 as that's what the net module uses - https://github.com/nodejs/node/blob/master/lib/net.js#L1327

[bnoordhuis]

LGTM with a comment about the test.

In the man pages for either mac or on Linux, I cannot find reference to the value 0 being special either.

It is on macos; 0 makes it default to kern.ipc.somaxconn (normally 128.)

[bnoordhuis]

Shouldn't the exception bubble up? Also, there seems to be no point in returning response.

[Trott]

Many relevant CI failures along the lines of:

16:51:36 not ok 1862 parallel/test-inspector-stress-http
16:51:36   ---
16:51:36   duration_ms: 120.69
16:51:36   severity: fail
16:51:36   exitcode: -15
16:51:36   stack: |-
16:51:36     timeout

Moving the test to sequential rather than parallel may fix it, or at least help a bit.

[eugeneo]

@Trott - I believe that was an old run. That timeout was a result of the 0 backlog on Linux.

Green CI: https://ci.nodejs.org/job/node-test-commit/17997/

happy to be wrong!

[eugeneo]

Test was updated, the CI is still green: https://ci.nodejs.org/job/node-test-pull-request/14529/

Will land now

[eugeneo]

Landed as 71059fb