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

Fix race condition in sandbox #113

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

Keno
Copy link
Contributor

@Keno Keno commented Sep 26, 2023

The current sandbox code has a race condition. If the child exits before the sigwait signal mask is setup, we will hang forever waiting for a signal. This has been observed in practice in Yggdrasil. I don't know that the sigwait here is supposed to do - plain waitpid should be fine, so try removing it to see if everything works.

If the sigwait does turn out to be necessary, we should be able to fix this by moving the signal mask setup to above the fork instead.

The current sandbox code has a race condition. If the child exits
before the sigwait signal mask is setup, we will hang forever waiting
for a signal. This has been observed in practice in Yggdrasil.
I don't know that the sigwait here is supposed to do - plain
waitpid should be fine, so try removing it to see if everything
works.

If the sigwait does turn out to be necessary, we should be able
to fix this by moving the signal mask setup to above the fork instead.
@staticfloat
Copy link
Collaborator

I'm pretty sure this code is vestigial from the olden days when this C code was used to setup a fake init for a QEMU-emulated environment for BinaryBuilder.jl. Because it was acting as init, it needed to react to SIGCHILD to properly take care of orphaned zombie processes. So I think removing it should be fine.

@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (949bf17) 78.68% compared to head (aa73670) 78.68%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #113   +/-   ##
=======================================
  Coverage   78.68%   78.68%           
=======================================
  Files           6        6           
  Lines         624      624           
=======================================
  Hits          491      491           
  Misses        133      133           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@staticfloat staticfloat merged commit a0a0950 into JuliaContainerization:main Sep 26, 2023
9 checks passed
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.

2 participants