Repository navigation
Conversation
|
Hi @chennaji9. Thanks for your PR. I'm waiting for a OpenAtom-Linyaps member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
closeParentEnds() closed the same descriptors as closeChildEnds(), leaking the parent-owned pipe ends after fork; close the stdin write end and the stdout/stderr read ends, which are the ends the parent owns per setupChildStdio(); add a unit test. Signed-off-by: 陈那几 <2078634206@qq.com>
the non-tty parent branch called closeParentEnds(), which used to (incorrectly) close the child-facing ends; now that closeParentEnds() actually closes the parent ends, the parent must call closeChildEnds() to drop its copies of the child-facing descriptors before adopting the parent ends for the forwarding loop. Signed-off-by: 陈那几 <2078634206@qq.com>
d4abd73 to
5bf5399
Compare
What does this PR do?
PipeSetsplits the six pipe descriptors between parent and child (persetupChildStdio()): the child owns the stdin READ end and the stdout/stderr WRITE ends; the parent owns the stdin WRITE end and the stdout/stderr READ ends.closeChildEnds()closed the correct three descriptors, butcloseParentEnds()was an exact copy of it — it closed the child-facing ends again and never closed the ends the parent actually owns. A parent calling it afterfork()leaked its own pipe ends.Two commits:
fix(utils): makecloseParentEnds()close the parent-owned ends (stdin write end, stdout/stderr read ends).fix(cli): the only caller, the non-tty parent branch ofCli::reuseContainer, calledcloseParentEnds()while actually needing to drop the parent's copies of the child-facing ends — switch it tocloseChildEnds(). (It worked before only because the oldcloseParentEnds()body was wrong in exactly that way.) The parent then adopts the parent-owned ends viareleaseWriteEnd()/releaseReadEnd()for the epoll forwarding loop, unchanged.Validation
PipeSet.CloseParentEndsKeepsChildEndsOpen: aftercloseParentEnds(),fcntl(F_GETFD)confirms the child-facing ends are still open while the parent-facing ends are closed.ctest -j4→ 737/737 passed (includes all existing Cli/utils tests — no regressions from thereuseContainerchange).🤖 Generated with Claude Code