This repository was archived by the owner on Feb 8, 2021. It is now read-only.
Fixes #520 #527 #536
Closed
Fixes #520 #527 #536
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
only assign pty path to one process named runv-shim whose lifecyle is same as the terminal. After did a lot of tests in my environment, I found that if multiple processes have opened the same pty, docker-containerd-shim wonn't get the close event of pty and will get stucked on waiting streem terminate forever. Because runv and runv-containerd don't have a controlling tty, when they opening the pty, it will become their controlling tty automatically. that's why it complained about "operation not permitted" in case which mentioned in bug hyperhq#520. AFAIK a pty can only become controlling tty of one process. Signed-off-by: frank yang <[email protected]>
Member
Contributor
|
runv-containerd is gone now after refactor, I don't think is pr is needed anymore. @yyb196 what do you think? And thank you for your contribution! |
Contributor
Author
|
i will check the new version of code later, what should i do to close this pr. @laijs |
Contributor
|
I am afraid conflict happens here in the PR. A rebase needed though. @yyb196 🍻 |
Contributor
|
still friendly ping @yyb196 🍻 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #520 #527
only assign pty path to one process named runv-shim whose lifecyle is same as the terminal.
After did a lot of tests in my environment, I found that if multiple processes
have opened the same pty, docker-containerd-shim wonn't get the close event
of pty and will get stucked on waiting streem terminate forever.
Because runv and runv-containerd don't have a controlling tty, when they
opening the pty, it will become their controlling tty automatically.
that's why it complained about "operation not permitted" in case which mentioned
in bug #520. AFAIK a pty can only become controlling tty of one process.
Signed-off-by: frank yang [email protected]