Skip to content

Fixed container init process not re-parent to youki main process#1637

Merged
utam0k merged 1 commit intoyouki-dev:mainfrom
yihuaf:yihuaf/fix-exec
Mar 11, 2023
Merged

Fixed container init process not re-parent to youki main process#1637
utam0k merged 1 commit intoyouki-dev:mainfrom
yihuaf:yihuaf/fix-exec

Conversation

@yihuaf
Copy link
Copy Markdown
Collaborator

@yihuaf yihuaf commented Mar 8, 2023

Fix #1601

In this PR, I decided to clone intermediate process as child and then clone the container init process as sibling of the intermediate process and child of youki main process. In this way, when intermediate process exits, the container init process will not re-parent to pid 1 and become zombie. The caller of youki can decide to use subreaper flag to adopt the container init process when youki main process exits.

This method cleaner because youki main process is guaranteed to be the parent of both intermediate process and container init process at all time. There is no extra re-parenting.

This PR is only part of the fix. We do need to revisit the detached and foreground mode. Not everything is fixed, but this PR should maintain the old behavior on anything that it does not fix.

@yihuaf yihuaf self-assigned this Mar 8, 2023
@yihuaf yihuaf marked this pull request as draft March 8, 2023 05:36
@yihuaf
Copy link
Copy Markdown
Collaborator Author

yihuaf commented Mar 8, 2023

See this comment for more discussion: #1601 (comment)

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 8, 2023

Codecov Report

Merging #1637 (9632c06) into main (ff5dba9) will increase coverage by 0.01%.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
+ Coverage   68.93%   68.95%   +0.01%     
==========================================
  Files         120      120              
  Lines       13156    13157       +1     
==========================================
+ Hits         9069     9072       +3     
+ Misses       4087     4085       -2     

@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch from cfb32ae to e9ef2ca Compare March 8, 2023 06:42
@yihuaf yihuaf marked this pull request as ready for review March 8, 2023 06:49
@yihuaf yihuaf requested review from Furisto, YJDoc2 and utam0k March 8, 2023 06:49
@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch 2 times, most recently from 5369ab7 to 9632c06 Compare March 8, 2023 06:57
@yihuaf yihuaf force-pushed the yihuaf/fix-exec branch from 9632c06 to 4fb1d41 Compare March 9, 2023 16:54
Signed-off-by: Eric Fang <yihuaf@unkies.org>
Copy link
Copy Markdown
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@utam0k utam0k merged commit ef5daba into youki-dev:main Mar 11, 2023
@yihuaf yihuaf deleted the yihuaf/fix-exec branch March 11, 2023 03:04
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.

Use clone(2) instead of fork(2)

3 participants