Repository navigation
Investigate test-child-process-fork-closed-channel-segfault #20836
Description
not ok 51 parallel/test-child-process-fork-closed-channel-segfault
---
duration_ms: 0.259
severity: fail
exitcode: 1
stack: |-
c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-closed-channel-segfault.js:70
throw err;
^
Error: write EMFILE
at ChildProcess.target._send (internal/child_process.js:741:20)
at ChildProcess.target.send (internal/child_process.js:625:19)
at Worker.send (internal/cluster/worker.js:40:28)
at Socket.<anonymous> (c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-closed-channel-segfault.js:36:16)
at Object.onceWrapper (events.js:273:13)
at Socket.emit (events.js:182:13)
at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1147:10)
Likely related:
not ok 52 parallel/test-child-process-fork-net
---
duration_ms: 0.354
severity: fail
exitcode: 1
stack: |-
PARENT: server listening
CHILD: server listening
CLIENT: connected
PARENT: got connection
CLIENT: connected
CHILD: got connection
CLIENT: closed
CHILD: got connection
CHILD: got connection
CLIENT: closed
CLIENT: connected
CLIENT: connected
CLIENT: closed
CLIENT: closed
PARENT: server closed
CHILD: got socket
CLIENT: got data
CLIENT: closed
testSocket, listening
events.js:167
throw er; // Unhandled 'error' event
^
Error: write EPIPE
at ChildProcess.target._send (internal/child_process.js:741:20)
at ChildProcess.target.send (internal/child_process.js:625:19)
at SocketListSend._request (internal/socket_list.js:20:16)
at SocketListSend.close (internal/socket_list.js:40:10)
at Server.close (net.js:1624:24)
at Socket.<anonymous> (c:\workspace\node-test-binary-windows\test\parallel\test-child-process-fork-net.js:179:16)
at Socket.emit (events.js:182:13)
at TCP._handle.close [as _onclose] (net.js:596:12)
Emitted 'error' event at:
at process.nextTick (internal/child_process.js:745:39)
at process._tickCallback (internal/process/next_tick.js:61:11)
Activity
gireeshpunathil commentedon May 19, 2018
For the first one,
EMFILEis predicted to occur according to this comment but not sure why it wasn't captured in the test.Trott commentedon May 20, 2018
@gireeshpunathil The test checks that
err.messageis'Channel closed'and it's not forEMFILE. Not sure if that's a recent change to the message or what. The code that checks for that string was added in #8954 by @santigimeno (when the test had a different name--it was subsequently renamed).@nodejs/testing @nodejs/platform-windows @nodejs/child_process
Trott commentedon May 20, 2018
(Interestingly, the string was
'channel closed'in the original PR but is now'Channel closed'with different capitalization.)BridgeAR commentedon May 21, 2018
And another one (again both tests failed - I wonder if the first test has influence on the second one).
https://ci.nodejs.org/job/node-test-binary-windows/17446/COMPILED_BY=vs2017,RUNNER=win2016,RUN_SUBSET=3/console
Trott commentedon May 21, 2018
One issue with
test-child-process-fork-netis that is uses bothconsole.log()andconsole.error(). The test runner does not preserve the interleaving of messages from stdout and stderr, so the messages as shown in CI are not the order they appear in when you run the test from the command line. PR to fix that aspect of things: #20873 A 👍 to fast-track that would be great.Trott commentedon May 22, 2018
Better output for test-child-process-fork-net (stdout and stderr properly interleaved):
Trott commentedon May 23, 2018
I tried to run Windows stress tests for this to see how often it fails but after compiling for a long time, the builds hit a three-minute timeout and don't run the tests. See https://ci.nodejs.org/job/node-stress-single-test/1858/nodes=win2016-1p-vs2017/console and https://ci.nodejs.org/job/node-stress-single-test/1857/nodes=win2016-1p-vs2017/console.
@nodejs/build
bzoz commentedon May 23, 2018
On my Win 10 box
test-child-process-fork-closed-channel-segfaultfails about 6% times aboutparallel/test-child-process-fork-netfails about once every 30k runs.apapirovski commentedon May 23, 2018
@bzoz any chance you could have a look at
test-http2-pipeand see if it still fails under load? (unrelated to this issue, of course) If so, could you build with--debug-http2 --debug-nghttp2and post the debugging info from a failed run to this issue: #20750It would really be appreciated!
bzoz commentedon May 23, 2018
@apapirovski sure, I'll take a look
apapirovski commentedon May 23, 2018
Just my 2c but this test has been flaky since its introduction, only very infrequently replicates the condition on the originally bugged version (Node 4.0.0) and is unlikely to occur exactly like this again. As such, I think we should scrap the test and move the one important check (
assert.strictEqual(worker.process.channel, null);) into another test that uses child_process in a similar manner.If we try to fix it yet again, it'll be a 6th PR to make this test less flaky. There are currently 5 separate error conditions that we ignore in this. We might be adding a sixth. IMO it's more trouble than it's worth.
5 remaining items
mmarchini commentedon May 26, 2018
Is
parallel/test-child-process-fork-netactually related or should we open another issue to fix it?gireeshpunathil commentedon May 27, 2018
@mmarchini - looking the failing stack, I believe it is the same issue. So you can apply the same patch as in #20973 over there too, if you are going for a PR. However, I have found issues with it, and please wait while we fix this.
mmarchini commentedon May 29, 2018
@gireeshpunathil do you mind if I open a PR to fix
parallel/test-child-process-fork-netas well?test: fix worker send error
test: fix worker send error