Skip to content

Use raw syscalls to avoid sporadic hangs#2425

Merged
utam0k merged 1 commit intoyouki-dev:mainfrom
jprendes:non-blocking-syscalls
Oct 12, 2023
Merged

Use raw syscalls to avoid sporadic hangs#2425
utam0k merged 1 commit intoyouki-dev:mainfrom
jprendes:non-blocking-syscalls

Conversation

@jprendes
Copy link
Copy Markdown
Contributor

The setgroups/setresgid/setresuid functions in nix/libc are not safe to call after clone/clone3.
These function use a blocking mechanism that synchronises across all threads in the process.
To do this libc (glibc/musl) needs to do some bookkeeping, which breaks on the new process after clone.
See the discussion in containerd/runwasi#347 for more details.

This PR worksaround that issue by using raw syscalls for setgroups/setresgid/setresuid.

This is generally not safe, as the raw syscalls only affect the calling thread and not the whole process.
It is safe to do so in this case because at the point when they are called in libcontainer the init process only has one thread.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 10, 2023

Codecov Report

Merging #2425 (32ffc4d) into main (8faff1c) will decrease coverage by 0.08%.
Report is 4 commits behind head on main.
The diff coverage is 0.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2425      +/-   ##
==========================================
- Coverage   66.01%   65.93%   -0.08%     
==========================================
  Files         133      133              
  Lines       16832    16851      +19     
==========================================
  Hits        11111    11111              
- Misses       5721     5740      +19     

Signed-off-by: Jorge Prendes <jorge.prendes@gmail.com>
@jprendes jprendes force-pushed the non-blocking-syscalls branch from e2f05e0 to 32ffc4d Compare October 11, 2023 10:04
@utam0k
Copy link
Copy Markdown
Member

utam0k commented Oct 11, 2023

@jprendes I appreciate your investigation and PR. Have you checked if this patch worked fine for runwasi?

@jprendes
Copy link
Copy Markdown
Contributor Author

@utam0k it seems to work. It is difficult to say as the original issue is sporadic.
But so far after a few runs I haven't gotten any failure with this change.

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.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants