[sxa]

No description provided.

[nodejs-github-bot]

Review requested:

• @nodejs/gyp

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[sxa]

Shared library build at https://ci.nodejs.org/job/node-test-commit-aix-shared-lib/nodes=aix73-power9/2617 worked with one exception - but that's much better than previously.

not ok 7282 addons/dlopen-ping-pong/test
  ---
  duration_ms: 280.95800
  severity: crashed
  exitcode: -4
  stack: |-
    process.dlopen: /home/iojs/build/workspace/node-test-commit-aix-shared-lib/nodes/aix73-power9/test/addons/dlopen-ping-pong/build/Release/binding.node
    module.exports.load: /home/iojs/build/workspace/node-test-commit-aix-shared-lib/nodes/aix73-power9/test/addons/dlopen-ping-pong/build/Release/ping.so
  ...

Noting that the test does have a block for Windows:

if (common.isWindows)
  common.skip('dlopen global symbol loading is not supported on this os.');

but in our case it does pass on AIX in the non-shared builds.

Incoming force push to update commit message, but no extra code changes.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[sxa]

reason for the failure may be related to the runtimelin linking options (or lack of them!) in the shared library builds. Latest commit will try to resolve that by adding this to the shared library clause:

'-Wl,-brtl',

Related: #15286

[sxa]

Second commit here updates the pingpong test so that it works as well. This could reasonably be a separate PR so if that is desired I can split it out. The functionality can be reviewed separately, which is why I have not squashed the PRs. This is the first time I've looked at this test but it seems quite unique in some of what it does, and happens to trip up on AIX's shared library scenario.

The test does three things: load binding.node with RTLD_GLOBAL, load ping.so, then call dlopen_pong from the binding via code in ping.so.

On AIX, ping.so was built with -Wl,-G, so it imported dlopen_pong at dlopen(ping.so) time (deferred binding). binding.node did export dlopen_pong in dump, but in the thin node + libnode shared build that import often did not resolve correctly at load time causing a (SIGILL, exit −4). In Release, assert in binding.cc is off [*], so a failed load could leave a null function pointer and you’d get the same kind of crash instead of a clear error.

With the fix:

1. ping.c on AIX: Look up dlopen_pong with dlsym(RTLD_DEFAULT, …) when dlopen_ping runs, which matches what RTLD_GLOBAL is supposed to do, without relying on -G import fixup when ping.so loads. dlsym(RTLD_DEFAULT, "dlopen_pong") skips the import-at-dlopen(ping.so) path: ping.so no longer needs -G for this symbol on AIX, and lookup happens explicitly when dlopen_ping runs, using the global symbol set RTLD_GLOBAL was meant to publish.
2. binding.gyp: no -Wl,-G on the AIX ping target, so ping.so doesn’t carry that fragile import.
3. binding.cc - throw on failure instead of assert, so failures show up as test errors, not −4.

[*] - Replacing assert with explicit checks that throw into JavaScript makes Release behave like a real test failure instead of a silent native crash.