Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: conn_sock: close stdout side when we don't close stdin #166

Closed
wants to merge 1 commit into from

Conversation

haircommander
Copy link
Collaborator

the main consumer of leave-stdin-open, cri-o, needs stdin to stay open, but also wants to consume all data from stdout.
It does not know when the output from stdout has ended unless conmon closes its side of the pipe.

the problem is, if we don't close masterfd_stdin, we also don't close masterfd_stdout, as they point to the same side pipe, which results in cri-o sitting and waiting for input forever.

instead, we need to close the stdout side of the pipe if we don't close masterfd_stdin

Signed-off-by: Peter Hunt pehunt@redhat.com

the main consumer of leave-stdin-open, cri-o, needs stdin to stay open, but also wants to consume all data from stdout.
It does not know when the output from stdout has ended unless conmon closes its side of the pipe.

the problem is, if we don't close masterfd_stdin, we also don't close masterfd_stdout, as they point to the same side pipe, which results in cri-o sitting and waiting for input forever.

instead, we need to close the stdout side of the pipe if we don't close masterfd_stdin

Signed-off-by: Peter Hunt <pehunt@redhat.com>
@haircommander haircommander changed the title conn_sock: close stdout side when we don't close stdin WIP: conn_sock: close stdout side when we don't close stdin May 29, 2020
@haircommander
Copy link
Collaborator Author

waiting on cri-o/cri-o#3787 to verify no breaking changes here (the test passed but I didn't test any others)

@mrunalp
Copy link
Collaborator

mrunalp commented Jun 2, 2020

cri-o/cri-o#3812 fixes the tests. We don't need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants