nix icon indicating copy to clipboard operation
nix copied to clipboard

Rethinking the interface for `fork`

Open kamalmarhubi opened this issue 8 years ago • 11 comments
trafficstars

I was writing a shell as an example for nix, and I realised that #555 makes a really good point: the code that runs in the child after fork(2) must not allocate if there is more than one thread. I don't know if fork should necessarily be made unsafe, but certainly we need to document better what is ok to do afterwards.

This is somewhat related to #90 in that there's an execution context where only limited operations should be performed. Ideally nix would force only correct usage, but at the same time there's a lot of value of staying close to the libc definitions of functions. (#190 seems connected here too.)

kamalmarhubi avatar Apr 17 '17 03:04 kamalmarhubi

Also see http://www.linuxprogrammingblog.com/threads-and-fork-think-twice-before-using-them

And the man page says:

After a fork() in a multithreaded program, the child can safely
call only async-signal-safe functions (see signal-safety(7)) until
such time as it calls execve(2).

This makes it sound like fork really really should be made unsafe: Spawning threads and calling non-signal-safe function is possible in safe Rust, therefore there must not be a safe way to do so after a fork. Instead, nix could provide safe utility functions that do common patterns like fork + execve in a safe way.

jonas-schievink avatar Jul 19 '17 14:07 jonas-schievink

This has come up in the standard library, too: https://github.com/rust-lang/rust/issues/39575

jonas-schievink avatar Jul 19 '17 14:07 jonas-schievink

You're right that fork is unsafe. But is it unsafe in the Rust sense? I think we should follow whatever decision rust-lang makes in the issue you linked.

asomers avatar Jul 19 '17 15:07 asomers

@asomers I read through some of the discussion there. The argument is that UB should be assumed to include memory unsafety by default. Specifically for fork() there are some more reasonings that should make this more likely.

Susurrus avatar Jul 19 '17 15:07 Susurrus

Is there any undefined behavior associated with calling async-signal-unsafe functions after forking? I can't find any. fork(2) doesn't mention any undefined behavior. Instead, it suggests not calling async-signal-unsafe functions in order to prevent deadlocks and the like.

asomers avatar Jul 20 '17 18:07 asomers

@jonas-schievink @asomers Did this get cleaned resolved with the merging of #695?

Susurrus avatar Dec 03 '17 04:12 Susurrus

I believe the conclusion was that fork is safe but (eg.) allocating afterwards can cause deadlocks, which is still a footgun.

I don't know if a deadlock will always be the worst that can happen. Also, deadlocks will only happen when forking while another thread is allocating memory (or holding another lock), making this an annoying-to-debug race.

To further the issue, std lib functions generally don't document if they allocate or which libc functions they call.

If you don't think it makes sense to provide a safer interface that prevents this, you can probably close this issue.

jonas-schievink avatar Dec 03 '17 09:12 jonas-schievink

Update: The conclusion of https://github.com/rust-lang/rust/issues/39575 was that before_exec should have been an unsafe function, and has been deprecated and replaced by an unsafe fn pre_exec. Thus fork should also be considered an unsafe operation.

jonas-schievink avatar Nov 16 '19 13:11 jonas-schievink

@jonas-schievink thanks for that update!

kamalmarhubi avatar Nov 21 '19 04:11 kamalmarhubi

The rustdocs for fork and forkpty forget to mention the restriction is lifted after execve is called, I think this is mildly confusing

interacsion avatar Jan 28 '25 21:01 interacsion

The rustdocs for fork and forkpty forget to mention the restriction is lifted after execve is called, I think this is mildly confusing

PR #2591 will fix this

SteveLauC avatar Feb 02 '25 09:02 SteveLauC