search

hyperhq / runv

Hypervisor-based Runtime for OCI
Apache License 2.0
826 stars 129 forks source link

Fixes #520 #527 #536

Closed yyb196 closed 7 years ago

yyb196 commented 7 years ago

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 yyb196@gmail.com

gnawux commented 7 years ago

@yyb196 we merged #537 , which aimed to improve the compatibility with the latest containerd and changed the cli much. Could you help check whether need to update this PR? Thank you very much.

laijs commented 7 years ago

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!

yyb196 commented 7 years ago

i will check the new version of code later, what should i do to close this pr. @laijs

allencloud commented 7 years ago

I am afraid conflict happens here in the PR. A rebase needed though. @yyb196 🍻

allencloud commented 7 years ago

still friendly ping @yyb196 🍻