child_process: fix handle passing with large payloads#14588
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added the child_process 
lib/internal/child_process.js Outdated
There was a problem hiding this comment.
This looks incorrect: it clears the variable on the first iteration but what if the NODE_HANDLE message is not in the first chunk? I would have expected to see this assignment in the if (message.cmd === 'NODE_HANDLE'){ block.
There was a problem hiding this comment.
Right, I think that makes more sense – done.
addaleaxforce-pushed the cluster-large-message-handle branch 2 times, most recently from 8f3c0e0 to 2f65c3fCompare
bnoordhuis left a comment
bnoordhuis left a comment There was a problem hiding this comment.
LGTM. Maybe add an assert that checks pendingHandle is null again after the loop.
jasnell commented Aug 3, 2017
Fix situations in which the handle passed along with a message that has a large payload and can’t be read entirely by a single `recvmsg()` call isn’t associated with the message to which it belongs. Fixes: nodejs#13778
addaleaxforce-pushed the cluster-large-message-handle branch from 2f65c3f to be0fed0Compare
addaleax commented Aug 7, 2017
So … the test here isn’t even flaky on Windows, it’s just always failing. Would anybody in @nodejs/platform-windows have a chance to look at what’s going wrong? |
addaleax commented Aug 7, 2017
Tried something new with help from @tniessen CI: https://ci.nodejs.org/job/node-test-commit/11599/ |
jasnell commented Aug 8, 2017
Landed in 611851d |
Fix situations in which the handle passed along with a message that has a large payload and can’t be read entirely by a single `recvmsg()` call isn’t associated with the message to which it belongs. PR-URL: #14588Fixes: #13778 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins commented Aug 16, 2017
This is going to need a manual backport for both v4 and v6 |
7 participants
Fix situations in which the handle passed along with a message
that has a large payload and can’t be read entirely by a single
recvmsg()call isn’t associated with the message to which itbelongs.
Fixes: #13778
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
child_process