Skip to content

Commit a62dbe9

Browse files
committed
Mitigate forking while having multiple threads.
If `fork()` is called when having multiple threads, the child thread only inherits the forking thread. Other threads simply vanish, stopping anywhere it was executing without any cleaning up. As a result, when using MMTk, the modbufs of other threads will not be flushed, and the next nursery GC will fail to keep some root-reachable objects alive, and the program will crash later. This commit mitigates this problem by destroying mutators of dead threads after forking. Concretely, the call of `rb_mmtk_destroy_mutator` is moved into `thread_cleanup_func` which is called not only when the thread exits normally, but also after forking to clean up dead threads. That ensures the modbufs are flushed. According to the POSIX API, if `fork()` is called while there are multiple threads, the child can only call async-signal-safe functions until calling execve. There are test cases (such as `test_autoload_fork`) that forks while having multiple threads, and there may be real-world applications doing so (see https://bugs.ruby-lang.org/issues/14634). They are all unsafe according to POSIX. So this commit only mitigates the problem, and programs that fork while having multiple threads are still technically incorrect. Fixes: #83
1 parent 8082532 commit a62dbe9

File tree

3 files changed

+36
-16
lines changed

3 files changed

+36
-16
lines changed

Diff for: internal/mmtk_support.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ void rb_mmtk_main_thread_init(void);
4444

4545
// Flushing and de-initialization
4646
void rb_mmtk_flush_mutator_local_buffers(MMTk_VMMutatorThread thread);
47-
void rb_mmtk_destroy_mutator(MMTk_VMMutatorThread cur_thread);
47+
void rb_mmtk_destroy_mutator(MMTk_VMMutatorThread cur_thread, bool at_fork);
4848

4949
// Object layout
5050
size_t rb_mmtk_prefix_size(void);

Diff for: mmtk_support.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -357,11 +357,16 @@ rb_mmtk_flush_mutator_local_buffers(MMTk_VMMutatorThread thread)
357357
}
358358

359359
void
360-
rb_mmtk_destroy_mutator(MMTk_VMMutatorThread cur_thread)
360+
rb_mmtk_destroy_mutator(MMTk_VMMutatorThread cur_thread, bool at_fork)
361361
{
362-
// Currently a thread can only destroy its own mutator.
363-
RUBY_ASSERT(cur_thread == GET_THREAD());
364-
RUBY_ASSERT(cur_thread->mutator_local == &rb_mmtk_mutator_local);
362+
if (!at_fork) {
363+
// A thread only destroys its own mutator when it exits normally (not at fork).
364+
// But after forking, only the forking thread continue to live in the child process.
365+
// The living thread will call this function to close the mutators of all dead threads.
366+
// So we skip the assertions at fork.
367+
RUBY_ASSERT(cur_thread == GET_THREAD());
368+
RUBY_ASSERT(cur_thread->mutator_local == &rb_mmtk_mutator_local);
369+
}
365370

366371
rb_mmtk_flush_mutator_local_buffers(cur_thread);
367372

Diff for: thread.c

+26-11
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,32 @@ thread_cleanup_func(void *th_ptr, int atfork)
515515
th->locking_mutex = Qfalse;
516516
thread_cleanup_func_before_exec(th_ptr);
517517

518+
#if USE_MMTK
519+
// Release MMTk-specific per-mutator structs.
520+
if (rb_mmtk_enabled_p()) {
521+
bool destroy_mutator = true;
522+
523+
if (!atfork) {
524+
// If called when a thread exits normally (not at fork),
525+
// the current thread should have mutator-local data,
526+
// and we will destroy the mutator.
527+
RUBY_ASSERT(th->mutator != NULL);
528+
RUBY_ASSERT(th->mutator_local != NULL);
529+
} else {
530+
// If called after fork, we visit all threads,
531+
// including the threads that have been "scheduled dead".
532+
// We only destry the mutator if not already destroyed.
533+
if (th->mutator == NULL) {
534+
destroy_mutator = false;
535+
}
536+
}
537+
538+
if (destroy_mutator) {
539+
rb_mmtk_destroy_mutator(th, atfork);
540+
}
541+
}
542+
#endif
543+
518544
/*
519545
* Unfortunately, we can't release native threading resource at fork
520546
* because libc may have unstable locking state therefore touching
@@ -779,17 +805,6 @@ thread_start_func_2(rb_thread_t *th, VALUE *stack_start)
779805
thread_cleanup_func(th, FALSE);
780806
VM_ASSERT(th->ec->vm_stack == NULL);
781807

782-
#if USE_MMTK
783-
// Release MMTk-specific per-mutator structs.
784-
// We are still holding the thread_sched, so other threads cannot inspect the following
785-
// per-mutator structs. It's safe for us to clean them up at this time.
786-
if (rb_mmtk_enabled_p()) {
787-
RUBY_ASSERT(th->mutator != NULL);
788-
RUBY_ASSERT(th->mutator_local != NULL);
789-
rb_mmtk_destroy_mutator(th);
790-
}
791-
#endif
792-
793808
if (th->invoke_type == thread_invoke_type_ractor_proc) {
794809
// after rb_ractor_living_threads_remove()
795810
// GC will happen anytime and this ractor can be collected (and destroy GVL).

0 commit comments

Comments
 (0)