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

Fork and safety #137

Closed
sunfishcode opened this issue Dec 4, 2021 · 1 comment
Closed

Fork and safety #137

sunfishcode opened this issue Dec 4, 2021 · 1 comment

Comments

@sunfishcode
Copy link
Member

This is motivated by #76, a port of std to rustix.

std calls fork and execve (via execvp) to launch child processes in some situations. Rustix's execve can allocate, to convert args to CString, and to nul-terminate the args array. But POSIX says that child processes of fork can't call anything async-signal-unsafe, including allocation. So how can we port std's fork and execve calls to Rustix?

This is std, so, unlike mustang, we can't assume anything special about the global allocator.

Rust doesn't require users to pick a global allocator which registers its locks with pthread_atfork. It isn't documented, and there are notable allocators today that don't always do it. Getting them to do it is tricky, as registering with pthread_atfork at the right time is tricky. And even when allocators do it, they're often on thin ice because malloc, pthread_mutex_lock, pthread_mutex_unlock, and other relevant functions aren't guaranteed to be async-signal-safe in libc.

It's looking like what we'd really need to do to make fork truly usable would be to define a new set of rules defining what Rust code and Rust allocator implementations need to do to play well with fork, and then work with independent projects to get them to follow the rules.

But even if we do that, and get all the independent projects to follow all the rules, the environment in the child between fork and execve will always be really awkward. No matter how safe we make things, it'll still have things like file locks inherited from the parent, MAP_SHARED regions shared with the parent, file descriptors dup'd from the parent and sometimes renumbered, and a whole copy of all the parent's memory, which can lead to many surprising situations. Related considerations are that fork also isn't a great fit for wasm programs, and therefore also WASI. And it's not portable to Windows and other non-Unix platforms. And in C, POSIX says that the child can only call async-signal-safe functions, but the Rust ecosystem in general doesn't document what's async-signal-safe and what's not. And, in general there are good reasons for wanting to move away from fork anyway.

So far, the overall direction in the rustix ecosystem has been to push for making fork safer and more usable, because that's rustix's overall API approach. However, the above issues motivate looking for other directions.

Another possible direction is to treat fork as an inherently limited tool. It's a tool to use with execve to launch child processes in specialized ways that posix_spawn isn't quite flexible enough to support, and nothing more. As such, it isn't as important to have a safe interface to fork and execve themselves, and to support general-purpose things like allocating or computing random numbers. We just need to support the things that Rust's std needs in its fork and execve code.

We could support that by adding a non-allocating version of execve and execvp to rustix::runtime, which are unsafe and take char** arguments and NUL-terminated arrays just like libc's execve does, and just like the Unix syscalls do. And similar for any other function Rust needs to be non-allocating between fork and exec (though at a first pass through the current code, I don't see any—Rust is already careful to avoid anything that allocates in this path).

That way, std can call rustix's fork and execve directly, rather than calling them through origin. This will mean not running any pthread_atfork handlers, but that seems to be ok, because posix_spawn says it's unspecified whether the fork handlers run, and we're thinking of fork as just a generalized posix_spawn use case.

Mustang, for its part, can then move its fork and at_fork out of origin and into c-scape. Perhaps initially it won't support pthread_atfork at all, but that could be added if needed for C compatibility.

sunfishcode added a commit that referenced this issue Dec 7, 2021
As discussed in #137, the new idea is that rustix won't attempt to make
`fork` and `execve` safe interfaces in general, and rustix's `execve` will be
non-allocating and have a raw C-oriented API. This will allow `execve`
to be usable within the child of a `fork` even if the global allocator isn't
`fork`-protected.
sunfishcode added a commit that referenced this issue Dec 7, 2021
As discussed in #137, the new idea is that rustix won't attempt to make
`fork` and `execve` safe interfaces in general, and rustix's `execve` will be
non-allocating and have a raw C-oriented API. This will allow `execve`
to be usable within the child of a `fork` even if the global allocator isn't
`fork`-protected.
sunfishcode added a commit that referenced this issue Dec 7, 2021
As discussed in #137, the new idea is that rustix won't attempt to make
`fork` and `execve` safe interfaces in general, and rustix's `execve` will be
non-allocating and have a raw C-oriented API. This will allow `execve`
to be usable within the child of a `fork` even if the global allocator isn't
`fork`-protected.
sunfishcode added a commit to sunfishcode/mustang that referenced this issue Dec 7, 2021
Move `fork` from origin to c-scape, and convert it back to using
`unsafe extern "C" fn()` hooks instead of safe boxed closures. See
bytecodealliance/rustix#137 for discussion.

Add an at_thread_exit_raw to origin, which allows thread destructors to
be registered without allocating, which is important since Rust will
register TLS destructors while threads are being constructed, before
parking_lot is prepared for the global allocator to take locks.

Update to the latest rustix, and the change from a safe execve to an
unsafe raw execveat. This similarly allows it to avoid allocating, which
is important as it's primarily called in the child of `fork`.

And switch to a branch of parking_lot with changes to allow parking_lot locks
to be used from within global allocator implementations.

These changes allow us to move the `#[global_allocator]` back to the mustang
crate, and restore the wee_alloc allocator option. More generally,
origin and c-scape should in theory now work with any global allocator
implementation.
sunfishcode added a commit to sunfishcode/mustang that referenced this issue Dec 21, 2021
Move `fork` from origin to c-scape, and convert it back to using
`unsafe extern "C" fn()` hooks instead of safe boxed closures. See
bytecodealliance/rustix#137 for discussion.

Add an at_thread_exit_raw to origin, which allows thread destructors to
be registered without allocating, which is important since Rust will
register TLS destructors while threads are being constructed, before
parking_lot is prepared for the global allocator to take locks.

Update to the latest rustix, and the change from a safe execve to an
unsafe raw execveat. This similarly allows it to avoid allocating, which
is important as it's primarily called in the child of `fork`.

And switch to a branch of parking_lot with changes to allow parking_lot locks
to be used from within global allocator implementations.

These changes allow us to move the `#[global_allocator]` back to the mustang
crate, and restore the wee_alloc allocator option. More generally,
origin and c-scape should in theory now work with any global allocator
implementation.
sunfishcode added a commit to sunfishcode/mustang that referenced this issue Dec 21, 2021
Move `fork` from origin to c-scape, and convert it back to using
`unsafe extern "C" fn()` hooks instead of safe boxed closures. See
bytecodealliance/rustix#137 for discussion.

Add an at_thread_exit_raw to origin, which allows thread destructors to
be registered without allocating, which is important since Rust will
register TLS destructors while threads are being constructed, before
parking_lot is prepared for the global allocator to take locks.

Update to the latest rustix, and the change from a safe execve to an
unsafe raw execveat. This similarly allows it to avoid allocating, which
is important as it's primarily called in the child of `fork`.

And switch to a branch of parking_lot with changes to allow parking_lot locks
to be used from within global allocator implementations.

These changes allow us to move the `#[global_allocator]` back to the mustang
crate, and restore the wee_alloc allocator option. More generally,
origin and c-scape should in theory now work with any global allocator
implementation.
@sunfishcode
Copy link
Member Author

This is now implemented in #138 and sunfishcode/mustang#84.

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

No branches or pull requests

1 participant