Some test cases fail when the node executable is built by `--shared` #18535

yhwang · 2018-02-02T18:56:42Z

When use --shared option in configure, it would build the node in shared lib for people who embed node in their applications. It also uses the node_main.cc to build the node executable with the shared lib. When using this executable to run our existing test suites, the following test cases fail:

Ubuntu 16.04:

parallel/test-child-process-fork-exec-path
parallel/test-module-loading-globalpaths
parallel/test-postmortem-metadata
parallel/test-process-external-stdio-close
parallel/test-process-external-stdio-close-spawn
parallel/test-stdout-close-catch
abort/test-abort-backtrace

Maybe some of them are not suitable for shared lib build, maybe some are bugs. Fixing them would help us to have a CI job to build node in shared lib and verify it. This would also help embedding users to make sure the shared lib work. And we can start from Linux platform.

Platform: Linux
Subsystem: test

The text was updated successfully, but these errors were encountered:

yhwang · 2018-02-02T19:02:19Z

I am going to start to check the root causes for these test cases. I will update my finding here and fix if needed. Of course, comment/suggestion are welcome.

richardlau · 2018-02-03T05:16:31Z

parallel/test-module-loading-globalpaths copies the node executable into a temporary directory and runs the copy as some of the global paths are relative to where the binary is. I would guess that with the shared lib the copied node executable is unable to find/load the shared lib.

yhwang · 2018-02-05T18:13:23Z

@richardlau

I would guess that with the shared lib the copied node executable is unable to find/load the shared lib.

You're right. And both parallel/test-child-process-fork-exec-path and parallel/test-module-loading-globalpaths share the same issue. In Linux platform, gyp uses rpath=$ORIGIN/lib.target to build the executable. One way to fix it is adding LD_LIBRARY_PATH (DYLD_LIBRARY_PATH for macOS, LIBPATH for AIX) into process.env. Both of them passed when I tried this locally.

When building the node with `--shared` option, the major output is the shared library. However, we still build a node executable which links to the shared lib. It's for testing purpose. When testing with the executable, some test cases move/copy the executable, change the relative path to the shared library and fail. Using lib path env would solve the issue. However, in macOS, need to change the install name for the shared library and use rpath in the executable. In AIX, `-brtl` linker option rebinds the symbols in the executable and addon modules could use them. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#18626 Refs: nodejs#18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

yhwang · 2018-02-16T17:46:44Z

For parallel/test-process-external-stdio-close and parallel/test-process-external-stdio-close-spawn, the close callback gets SIGPIPE. So it would be pipe related issue, i.e. not able to write/receive data from the pipe. I will dig into that. If someone knows the possible cause, please chime in. Thanks.

When building the node with `--shared` option, we need to verify the symbols in shared lib instead of executable. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]>

When building the node with `--shared` option, the major output is the shared library. However, we still build a node executable which links to the shared lib. It's for testing purpose. When testing with the executable, some test cases move/copy the executable, change the relative path to the shared library and fail. Using lib path env would solve the issue. However, in macOS, need to change the install name for the shared library and use rpath in the executable. In AIX, `-brtl` linker option rebinds the symbols in the executable and addon modules could use them. Signed-off-by: Yihong Wang <[email protected]> PR-URL: #18626 Refs: #18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

When building the node with `--shared` option, we need to verify the symbols in shared lib instead of executable. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #18806 Refs: #18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

When using shared lib build, the binary path in the stack frames points to shared lib. Change the checking criteria in the test case to match that. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]>

For shared lib build, we leave the signal handling for embedding users. In these two test cases: - `parallel/test-process-external-stdio-close-spawn` - `parallel/test-process-external-stdio-close` The pipe is used for stdout and is destroied before child process uses it for logging. So the node executble that uses shared lib build receives SIGPIPE and the child process ends. This change ignores the SIGPIPE in node_main.cc for shared lib case. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]>

When using shared lib build, the binary path in the stack frames points to shared lib. Change the checking criteria in the test case to match that. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#19213 Refs: nodejs#18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

For shared lib build, we leave the signal handling for embedding users. In these two test cases: - `parallel/test-process-external-stdio-close-spawn` - `parallel/test-process-external-stdio-close` The pipe is used for stdout and is destroied before child process uses it for logging. So the node executble that uses shared lib build receives SIGPIPE and the child process ends. This change ignores the SIGPIPE in node_main.cc for shared lib case. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #19211 Refs: #18535 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

yhwang · 2018-03-13T00:45:52Z

kicked off a Linux CI to verify the current master code base with shared lib build:
https://ci.nodejs.org/job/node-test-commit-linux-as-shared-lib/5/nodes=ubuntu1404-64/
I will update the result later

yhwang · 2018-03-13T02:18:20Z

All tests passed in the CI job above. Seems #19211 fixed parallel/test-stdout-close-catch as well. Next step is to enable the shared-lib CI job (only Linux) for regular PR CI. I think it's time to close this issue.

When using shared lib build, the binary path in the stack frames points to shared lib. Change the checking criteria in the test case to match that. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #19213 Refs: #18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

For shared lib build, we leave the signal handling for embedding users. In these two test cases: - `parallel/test-process-external-stdio-close-spawn` - `parallel/test-process-external-stdio-close` The pipe is used for stdout and is destroied before child process uses it for logging. So the node executble that uses shared lib build receives SIGPIPE and the child process ends. This change ignores the SIGPIPE in node_main.cc for shared lib case. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #19211 Refs: #18535 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

When using shared lib build, the binary path in the stack frames points to shared lib. Change the checking criteria in the test case to match that. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #19213 Refs: #18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

For shared lib build, we leave the signal handling for embedding users. In these two test cases: - `parallel/test-process-external-stdio-close-spawn` - `parallel/test-process-external-stdio-close` The pipe is used for stdout and is destroied before child process uses it for logging. So the node executble that uses shared lib build receives SIGPIPE and the child process ends. This change ignores the SIGPIPE in node_main.cc for shared lib case. Refs: #18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: #19211 Refs: #18535 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

When building the node with `--shared` option, the major output is the shared library. However, we still build a node executable which links to the shared lib. It's for testing purpose. When testing with the executable, some test cases move/copy the executable, change the relative path to the shared library and fail. Using lib path env would solve the issue. However, in macOS, need to change the install name for the shared library and use rpath in the executable. In AIX, `-brtl` linker option rebinds the symbols in the executable and addon modules could use them. Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#18626 Refs: nodejs#18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>

When building the node with `--shared` option, we need to verify the symbols in shared lib instead of executable. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#18806 Refs: nodejs#18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Yuta Hiroto <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

When using shared lib build, the binary path in the stack frames points to shared lib. Change the checking criteria in the test case to match that. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#19213 Refs: nodejs#18535 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

For shared lib build, we leave the signal handling for embedding users. In these two test cases: - `parallel/test-process-external-stdio-close-spawn` - `parallel/test-process-external-stdio-close` The pipe is used for stdout and is destroied before child process uses it for logging. So the node executble that uses shared lib build receives SIGPIPE and the child process ends. This change ignores the SIGPIPE in node_main.cc for shared lib case. Refs: nodejs#18535 Signed-off-by: Yihong Wang <[email protected]> PR-URL: nodejs#19211 Refs: nodejs#18535 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>