src,worker: display remaining handles if uv_loop_close fails

[addaleax]

This is refactoring our native stack trace code, and adding support for a better error message when a uv_loop_close() for a worker fails because there are still active handles.

Example output in that case (taken from test failures on the current canary branch while looking into nodejs/node-v8#66):

uv loop at [0x5623f5b32950] has active handles
[0x5623f5b13360] async
	Close callback: (nil) 
	Data: 0x5623f5b343e0 
	(First field): 0x5623f4dbf000 vtable for node::PerIsolatePlatformData [out/Release/node]
out/Release/node[28598]: ../src/debug_utils.cc:217:void node::CheckedUvLoopClose(uv_loop_t*): Assertion `0 && "uv_loop_close() while having open handles"' failed.
 1: node::Abort() [out/Release/node]
[ ... stack trace ... ]

As opposed to the previous error message:

out/Release/node[31550]: ../src/node_worker.cc:307:virtual node::worker::Worker::~Worker(): Assertion `(uv_loop_close(&loop_)) == (0)' failed.
 1: node::Abort() [out/Release/node]
[ ... stack trace ... ]

Checklist

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

[nodejs-github-bot]

@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/28/pipeline

[addaleax]

Full CI: https://ci.nodejs.org/job/node-test-commit/19128/

[TimothyGu]

LGTM when the testing issues get ironed out.

[addaleax]

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

[Trott]

Some failures in Alpine Linux on CI.

Re-run: https://ci.nodejs.org/job/node-test-pull-request/15385/

[Trott]

Same failures again on Alpine.

20:55:58 not ok 2114 abort/test-addon-uv-handle-leak
20:55:58   ---
20:55:58   duration_ms: 0.955
20:55:58   severity: fail
20:55:58   exitcode: 1
20:55:58   stack: |-
20:55:58     assert.js:270
20:55:58         throw err;
20:55:58         ^
20:55:58     
20:55:58     AssertionError [ERR_ASSERTION]: uv loop at [0x562f2861e130] has active handles
20:55:58     [0x562f286a6080] timer
20:55:58     	Close callback: 0x7f99924f80f0 
20:55:58     	Data: 0x7f99926f9060 
20:55:58     [0x562f286ba640] timer
20:55:58     	Close callback: 0x7f99924f80f0 
20:55:58     	Data: 0 
20:55:58     [0x562f286ba700] timer
20:55:58     	Close callback: 0x7f99924f80f0 
20:55:58     	Data: 0x42 
20:55:58     /home/iojs/build/workspace/node-test-commit-linux/nodes/alpine-latest-x64/out/Release/node[4614]: ../src/debug_utils.cc:219:void node::CheckedUvLoopClose(uv_loop_t*): Assertion `0 && "uv_loop_close() while having open handles"' failed.
20:55:58     
20:55:58         at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/alpine-latest-x64/test/abort/test-addon-uv-handle-leak.js:53:5)
20:55:58         at Module._compile (internal/modules/cjs/loader.js:702:30)
20:55:58         at Object.Module._extensions..js (internal/modules/cjs/loader.js:713:10)
20:55:58         at Module.load (internal/modules/cjs/loader.js:612:32)
20:55:58         at tryModuleLoad (internal/modules/cjs/loader.js:551:12)
20:55:58         at Function.Module._load (internal/modules/cjs/loader.js:543:3)
20:55:58         at Function.Module.runMain (internal/modules/cjs/loader.js:744:10)
20:55:58         at startup (internal/bootstrap/node.js:267:19)
20:55:58         at bootstrapNodeJSCore (internal/bootstrap/node.js:600:3)
20:55:58   ...

[addaleax]

Yeah, this is still in progress … do we have any way of detecting musl libc? It would be a shame to skip that test part for all of Linux…

[bnoordhuis]

Apropos checking for musl libc, it's complicated...

You can check ldd node and see if there's a line about libc.so:

• If not, it's probably musl, statically linked.

• If yes, check if nm /path/to/libc.so prints gnu_get_libc_version. If not, it's probably musl.

It's easier to check if os.hostname() is one of the alpine buildbots, or if /etc/alpine-release exists.

That's not strictly correct (all alpine is musl but not all musl is alpine) but it's much less brittle.

[bnoordhuis]

USE(...)

[addaleax]

Done!

[bnoordhuis]

Why not ... & ~(...)? Then you don't have to suppress the linter.

[addaleax]

I’m getting a déjà vu with that review comment … 😄 I’ve switched it for now, but I really prefer &~ for masking operations

[bnoordhuis]

I'm surprised this works; passing in no flags is technically an error (they're non-zero on linux and macos.)

[addaleax]

@bnoordhuis I guess this could be made MS_ASYNC? Reading the Linux man page, it seems like that’s how Linux interprets 0 anyway …

I’m switching and seeing how that fares in CI.

[addaleax]

I’ve tried the ldd + nm suggestion – let’s see how that goes, if not, we can just go for one of the simpler options.

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

(Edit: 15 green ✔️ in a row, not even any flaky tests … 😍)

[addaleax]

Landed in 8e2d33f, aa2304b

Releasers: The first commit can be backported to LTS.