Move fork from origin to c_scape

[sunfishcode]

After a fork, the child only has one thread, and it looks like all the
other threads are suspended. This may leave behind locks in locked
states and cause deadlocks, however deadlocks are not considered unsafe.
Consequently, fork doesn't need to be unsafe.

This also changes the on_fork callback functions to be
Box<Fn() + Send> so that it's safe to call them, and adds code to
c-scape and non-mustang targets to wrap them as needed.

[bjorn3]

Making fork safe would make calling memmap2::MmapMut::map_mut() on a file created using create_memfd and closed after mapping (to avoid the usual UB of mmap) unsound. map_mut() uses MAP_SHARED, which would be shared between the original and forked process, yet it allows you to get a mutable reference on both sides of the fork. In addition using something like sqlite on both sides of the fork will likely lead to data corruption, potentially causing sqlite to invoke UB, thus making any sqlite wrapper unsound. I am pretty sure there are many other cases of code that can only run on one side of the fork without causing UB.

[sunfishcode]

Making fork safe would make calling memmap2::MmapMut::map_mut() on a file created using create_memfd and closed after mapping (to avoid the usual UB of mmap) unsound. map_mut() uses MAP_SHARED, which would be shared between the original and forked process, yet it allows you to get a mutable reference on both sides of the fork.

Should it be the responsibility of fork callers to avoid this hazard, or map_mut callers? One could plausibly make arguments either way. My habit is to think of mmap as so frought with unsafety that I default to putting responsibility on map_mut. But I can see how it could be harder for map_mut callers to prevent this hazard than for fork callers. I'll need to think about this more.

In addition using something like sqlite on both sides of the fork will likely lead to data corruption, potentially causing sqlite to invoke UB, thus making any sqlite wrapper unsound. I am pretty sure there are many other cases of code that can only run on one side of the fork without causing UB.

Can you say more about what the hazard is here? If the sqlite bindings expose a safe API, we should be able to make the same assumptions we do about Rust code. Or does it use MAP_SHARED or something else which is implicitly shared by a fork?

[bjorn3]

If the sqlite bindings expose a safe API, we should be able to make the same assumptions we do about Rust code. Or does it use MAP_SHARED or something else which is implicitly shared by a fork?

Normally sqlite locks the database file. A fork can happen after the database was locked, in which case we now got two competing sqlite instances fighting for the same database. The resulting corruption itself could lead to UB inside the sqlite code, or a TOCTOU could happen between checking that the file contents are sane and actually using it. (for example because the compiler assumes that the memory doesn't change behind the back of the current thread due to data races being UB) Sqlite supports using mmap, though as an optional feature. The same argument could be made for any database that uses mmap as they have to use MAP_SHARED to make their changes visible in the database file in the first place.

[bjorn3]

The new doc comment LGTM. Didn't closely review the rest of the PR.

[sunfishcode]

I think in theory there are ways these safety concerns could be pushed to other places. But ultimately, fork is a really low-level and special API, so it's not worth pushing this too much. I've now marked it as unsafe and described the conditions. Thanks for bringing these up!

[sunfishcode]

Thinking about this more, I've now written bytecodealliance/rustix#137 to organize my thoughts, and point to a new overall direction here.

[sunfishcode]

32-bit arm is failing because parking_lot's Mutex uses std::time::Instant::now to implement fairness, which takes a lock on 32-bit arm to ensure monotonicity because it doesn't have 64-bit atomics and can't guarantee that CLOCK_MONOTONIC is actually monotonic, so it then deadlocks on the bucket lock.

[sunfishcode]

I think the thing to do for now is to temporarily disable 32-bit arm support. I expect the available options with parking_lot will evolve over time, and we can re-evaluate 32-bit arm support when we have other options.