test: investigate flakiness of `parallel/test-async-wrap-getasyncid`

[refack]

• Version: master
• Platform: Windows smartos15-64 ++
• Subsystem: test,async-hooks

file:

https://github.com/nodejs/node/blob/master/test/parallel/test-async-wrap-getasyncid.js

falakings

1. https://ci.nodejs.org/job/node-test-binary-windows/8471/
2. https://ci.nodejs.org/job/node-test-binary-windows/8362/
3. https://ci.nodejs.org/job/node-test-binary-windows/8402/

TAP output

2	parallel/test-async-wrap-getasyncid	
duration_ms	0.232
severity	fail
stack	
Mismatched <anonymous> function calls. Expected 1, actual 0.
    at Object.<anonymous> (c:\workspace\node-test-binary-windows\RUN_SUBSET\3\VS_VERSION\vs2015\label\win2008r2\test\parallel\test-async-wrap-getasyncid.js:155:42)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:144:16)
    at bootstrap_node.js:561:3
(node:5848) [DEP0064] DeprecationWarning: tls.createSecurePair() is deprecated. Please use tls.Socket instead.

Hypothesis

Seems like a net.server sometimes doesn't complete it's creation before the test exits...
const server = net.createServer(common.mustCall((socket) => {

[refack]

Stress CI: https://ci.nodejs.org/job/node-stress-single-test/1219/nodes=win10/

[refack]

@addaleax @AndreasMadsen is it possible that the anonymous scope + eager gc calls, cleans up the server before it has a chance to (h)->flags = UV__HANDLE_REF?

[addaleax]

As far as I understand common.mustCall(), all missed functions are listed on exit. If you were right, that would mean that the .listen() handler wouldn’t get called, but it’s not listed so it seems safe to assume it does get called?

[AndreasMadsen]

The only explanation that makes sense to me, is that the client connects, sends data, and shutdown itself before the actual server received any connection. Thus the libuv loop doesn't know that there a connection incoming and will just exit the process.

We could wait until the server received its connection before running handle.shutdown() or alternatively simply remove the common.mustCall from net.createServer().

In any case, the net.createServer() should actually not call socket.end(); or socket.destroy(); since that creates a race conditon between socket.destroy(); and handle.shutdown().

[refack]

safe to assume it does get called?

Yep...

We could wait until the server received its connection before running handle.shutdown()

👍 handle.shutdown() only in req.on('end')
I'm on it.

[AndreasMadsen]

handle.shutdown() only in req.on('end')

We need to wait for both server.on('connection') and wreq.oncomplete. In particular, req.on('end') will not happen at all before handle.shutdown().

[AndreasMadsen]

@refack was this issue solved? I don't remember.

[refack]

I think someone somewhere said something about #12854 might solve this... 🤔
Anyway I did not see it flake in a while.
Running another stress https://ci.nodejs.org/job/node-stress-single-test/nodes=win10/1232/

[AndreasMadsen]

I think the script path was spelled wrong, rerunning: https://ci.nodejs.org/job/node-stress-single-test/1240/

[refack]

✔️ More rigorous stress (-j 16 parallel/test-async-wrap-getasyncid X 100): https://ci.nodejs.org/job/node-stress-single-test/1242/nodes=win10/