Skip to content

Conversation

@nivkner
Copy link
Contributor

@nivkner nivkner commented Oct 24, 2021

when a thread calls fork(2), if an other thread acquires the allocator lock,
the child process would be deadlocked on any attempt to allocate, since only the calling thread persists in the new process.

this feature allows allocations to work as intended after the fork, by locking the global allocator before the fork and release the lock afterwards in the forking thread, using pthread_atfork.

@alexcrichton
Copy link
Owner

Thanks for the PR, but have you run into issues missing this code? I was under the impression that after fork the standard libray at least tried to not allocate, but if it does allocate and this is configured as the global allocator this seems necessary.

With this PR though I would prefer a solution that doesn't use an external dependency. I also don't think that using once_cell is appropriate since without the standard libray it probably uses a spin lock which is not appropriate.

@nivkner
Copy link
Contributor Author

nivkner commented Oct 25, 2021

this would be used by mustang, which replaces the parts of libc used by the standard library, with implementations based on rsix.

certain functions in libc, such as chdir(2), are specified to be async-signal-safe, which allows them, among other things, to be used after a fork.
therefore std might use them in the implementation of std::process.

in rsix, however, some of these functions allocate and therefore would not be usable in this context without protecting the allocator from the aforementioned deadlock.

will change the implementation to use ptheads

@sunfishcode
Copy link
Contributor

Since acquire_global_lock is already acquiring a global lock, would it work to replace the once_cell here with a plain static mut bool that's read and written only while the lock is held?

@nivkner nivkner force-pushed the fork_safe branch 2 times, most recently from 5a3bc7f to 9268eff Compare October 25, 2021 20:50
@nivkner nivkner changed the title add feature to make allocations safe in fork make allocations safe in fork Oct 26, 2021
@alexcrichton
Copy link
Owner

Ok this seems reasonable enough to me!

@alexcrichton alexcrichton merged commit 0a1c253 into alexcrichton:master Oct 26, 2021
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.

4 participants