Repository navigation
node-daily-v14.x-staging is failing on smartos #3154
Description
Extracted out of #3131:
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-v14.x-staging/ has been failing since 6 December. Cursory glance appears to be:
On macOS
(Fixed in #3131.)
On smartos there's one or more stray Node.js process being left behind:
11:29:41 ps awwx | grep Release/node | grep -v grep | cat
11:29:42 574381 ? S 0:01 /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/node -e setInterval(()=>{}, 99)
11:29:42 574413 ? S 0:01 /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/node -e setInterval(()=>{}, 99)
11:29:42 make[1]: *** [Makefile:515: test-ci] Error 1
Needs further investigation. The CI jobs are configured to fail if leftover Node.js processes are detected. As far as I can tell the tests themselves have passed. This may have started after the recent migration of the smartos machines (cc https://github.com/orgs/nodejs/teams/platform-smartos @bahamat @sxa ) -- I had clean CI's for Node.js 14.21.2 on December 8: https://ci.nodejs.org/job/node-test-commit-smartos/46911/nodes=smartos20-64/
Activity
richardlau commentedon Jan 19, 2023
I've narrowed down
node -e setInterval(()=>{}, 99)to coming fromtest-child-process-exec-abortcontroller-promisified. This test is passing but leaving behind Node.js child processes.Curiously this test doesn't run the
nodechild process on Linux and instead usessleep 2m. This was done in nodejs/node#37572 -- prior to that PR all platforms usedsleep 2mapart from Windows (which was flaky). If I switch the test to usesleep 2mon SmartOS the test still passes but doesn't leave behind anynodechild processes (no surprise since it's not creating any) but also is not leaving behind anysleepprocesses.I'm not entirely sure why we didn't have the child processes being left behind on smartos before the migration -- perhaps this is something timing related? We're not seeing the leftover child processes on
mainor current but are consistently seeing them on Node.js 14 and 16 SmartOS CI runs when we were not prior to the migration.I'm going to open a PR to switch back to
sleep 2mfor the test on POSIX platforms to unblock the Node.js 14 and 16 CI.richardlau commentedon Jan 19, 2023
PR: nodejs/node#46276
bahamat commentedon Jan 19, 2023
Looking at the issue, and the proposed fix, I think this is the correct approach. I can't mark approval on the PR, so I'm noting it here instead.
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes
test: avoid left behind child processes