Skip to content

Commit 87cfcff

Browse files
Only report gc_in_progress if the world is stopped
MMTk core's implementation of `gc_in_progress` returns true when a GC is requested in MMTk, and completes right at the end of the process. this means it returns true while the mutators are still running before they've fully stopped, so there's a window there where it's not safe to do things that require `gc_in_progress` to be false. We're seeing this on Ractors where Ractor.take is trying to use rb_objspace_each_objects, which checks that a GC is not running, but it's reporting true, because a GC has just been requested. This commit doesn't rely on MMTk to tell us whether a GC is running, but uses an AtomicBool in the Ruby binding to track whether or not we've fully stopped the mutators. Realistically I think that rb_gc_impl_during_gc_p is badly named now that we have GC running in seperate threads and are trying to move towards concurrent GC, but we should tackle this later
1 parent 6dd9db5 commit 87cfcff

2 files changed

Lines changed: 9 additions & 1 deletion

File tree

gc/mmtk/src/api.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ pub extern "C" fn mmtk_gc_enabled_p() -> bool {
269269

270270
#[no_mangle]
271271
pub extern "C" fn mmtk_gc_in_progress_p() -> bool {
272-
crate::mmtk().gc_in_progress()
272+
crate::collection::mutators_stopped()
273273
}
274274

275275
// =============== Object allocation ===============

gc/mmtk/src/collection.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ use std::sync::atomic::Ordering;
1818
use std::thread;
1919

2020
static CURRENT_GC_MAY_MOVE: AtomicBool = AtomicBool::new(false);
21+
static MUTATORS_STOPPED: AtomicBool = AtomicBool::new(false);
22+
23+
pub fn mutators_stopped() -> bool {
24+
MUTATORS_STOPPED.load(Ordering::SeqCst)
25+
}
2126

2227
pub struct VMCollection {}
2328

@@ -31,6 +36,7 @@ impl Collection<Ruby> for VMCollection {
3136
F: FnMut(&'static mut mmtk::Mutator<Ruby>),
3237
{
3338
(upcalls().stop_the_world)();
39+
MUTATORS_STOPPED.store(true, Ordering::SeqCst);
3440

3541
if crate::mmtk().get_plan().current_gc_may_move_object() {
3642
CURRENT_GC_MAY_MOVE.store(true, Ordering::Relaxed);
@@ -48,6 +54,8 @@ impl Collection<Ruby> for VMCollection {
4854
}
4955

5056
fn resume_mutators(_tls: VMWorkerThread) {
57+
MUTATORS_STOPPED.store(false, Ordering::SeqCst);
58+
5159
if CURRENT_GC_MAY_MOVE.load(Ordering::Relaxed) {
5260
(upcalls().after_updating_jit_code)();
5361
}

0 commit comments

Comments
 (0)