Update Ruby for the new GC disable API

gc.c

@@ -5894,6 +5894,29 @@ rb_gc_after_fork(rb_pid_t pid)
     rb_gc_impl_after_fork(rb_gc_get_objspace(), pid);
 }
 
+bool
+rb_gc_during_gc_could_malloc_region_start()
+{
+    // Do nothing when using MMTk.
+    WHEN_USING_MMTK({
+        return false;
+    })
+
+    return rb_gc_disable_no_rest();
+}
+
+void rb_gc_during_gc_could_malloc_region_end(bool already_disabled)
+{
+    // Do nothing when using MMTk.
+    WHEN_USING_MMTK({
+        return;
+    })
+
+    if (already_disabled == Qfalse) {
+        rb_gc_enable();
+    }
+}
+
 bool
 rb_gc_obj_shareable_p(VALUE obj)
 {

gc/default/default.c

@@ -1635,7 +1635,7 @@ bool
 rb_gc_impl_gc_enabled_p(void *objspace_ptr)
 {
     WHEN_USING_MMTK({
-        return mmtk_is_collection_enabled();
+        return rb_mmtk_is_collection_enabled();
     })
 
     rb_objspace_t *objspace = objspace_ptr;
@@ -1646,7 +1646,7 @@ void
 rb_gc_impl_gc_enable(void *objspace_ptr)
 {
     WHEN_USING_MMTK({
-        mmtk_enable_collection();
+        rb_mmtk_enable_collection();
         return;
     })
 
@@ -1659,7 +1659,7 @@ void
 rb_gc_impl_gc_disable(void *objspace_ptr, bool finish_current_gc)
 {
     WHEN_USING_MMTK({
-        mmtk_disable_collection();
+        rb_mmtk_disable_collection();
         return;
     })
 
@@ -3554,7 +3554,7 @@ rb_gc_impl_shutdown_call_finalizer(void *objspace_ptr)
         rb_gc_set_obj_free_on_exit_started();
 
         // Disable GC like the default GC does.
-        mmtk_disable_collection();
+        rb_mmtk_disable_collection();
 
         // Running data/file finalizers on exit, the MMTk style.
         // When using MMTk, we maintain a list of obj_free candidates in the Rust code,
@@ -7542,10 +7542,10 @@ rb_gc_impl_start(void *objspace_ptr, bool full_mark, bool immediate_mark, bool i
     rb_objspace_t *objspace = objspace_ptr;
 #if USE_MMTK
     if (rb_mmtk_enabled_p()) {
-        // Note: GC.start will initiates garbage collection even if manually disabled.
-        // Therefore, we need to force GC.
-        // We do a full-heap GC if full_mark is true.  In StickyImmix this may or may not trigger defragmentation.
-        // There is currently no way to force a defragmentation GC.
+        // Note: In CRuby, manual invocations of GC.start will still initiate garbage collection,
+        // but in MMTk, disabling GC means GC cannot be triggered in any way.
+        // The following statement may not necessarily trigger GC.
+        // TODO: Find a way to reconcile them.
         mmtk_handle_user_collection_request(GET_THREAD(), true, full_mark);
 
         gc_finalize_deferred(objspace);

internal/gc.h

@@ -175,10 +175,10 @@ struct rb_gc_object_metadata_entry {
  * necessary. */
 #define DURING_GC_COULD_MALLOC_REGION_START() \
     assert(rb_during_gc()); \
-    VALUE _already_disabled = rb_gc_disable_no_rest()
+    VALUE _already_disabled = rb_gc_during_gc_could_malloc_region_start()
 
 #define DURING_GC_COULD_MALLOC_REGION_END() \
-    if (_already_disabled == Qfalse) rb_gc_enable()
+    rb_gc_during_gc_could_malloc_region_end(_already_disabled)
 
 /* gc.c */
 RUBY_ATTR_MALLOC void *ruby_mimmalloc(size_t size);
@@ -278,6 +278,10 @@ void rb_gc_update_values(long n, VALUE *values);
 const char *rb_gc_active_gc_name(void);
 int rb_gc_modular_gc_loaded_p(void);
 
+/* Defined in gc.c; used by object_tracing.c */
+bool rb_gc_during_gc_could_malloc_region_start();
+void rb_gc_during_gc_could_malloc_region_end(bool already_disabled);
+
 RUBY_SYMBOL_EXPORT_END
 
 static inline VALUE

internal/mmtk.h

@@ -220,9 +220,9 @@ void mmtk_prepare_to_fork(void);
 
 void mmtk_after_fork(MMTk_VMThread tls);
 
-void mmtk_enable_collection(void);
+bool mmtk_enable_collection(void);
 
-void mmtk_disable_collection(void);
+bool mmtk_disable_collection(void);
 
 bool mmtk_is_collection_enabled(void);
 

internal/mmtk_support.h

@@ -178,6 +178,14 @@ void rb_mmtk_gc_probe_slowpath(bool enter);
 // xmalloc accounting
 void rb_mmtk_xmalloc_increase_body(size_t new_size, size_t old_size);
 
+// Disabling / enabling GC
+void rb_mmtk_disable_collection();
+void rb_mmtk_enable_collection();
+bool rb_mmtk_is_collection_enabled();
+
+// Block for GC
+void rb_mmtk_block_for_gc(MMTk_VMMutatorThread tls);
+
 // Commandline options parsing
 void rb_mmtk_pre_process_opts(int argc, char **argv);
 void rb_mmtk_post_process_opts(const char *arg);

mmtk_support.c

@@ -112,11 +112,19 @@ struct rb_mmtk_xmalloc_accounting{
     size_t malloc_total;
 } rb_mmtk_xmalloc_accounting_t;
 
+// States for disabling GC
+struct RubyMMTKGCDisablingState {
+    // This mutex only protects the `gc_is_disabled` field.
+    pthread_mutex_t mutex;
+    bool gc_is_disabled;
+};
+
 struct RubyMMTKGlobal {
     pthread_mutex_t mutex;
     pthread_cond_t cond_world_stopped;
     pthread_cond_t cond_world_started;
     rb_atomic_t mutator_blocking_count;
+    struct RubyMMTKGCDisablingState gc_disabling_state;
     unsigned int fork_hook_vm_lock_lev;
     bool world_stopped;
     size_t start_the_world_count;
@@ -125,6 +133,10 @@ struct RubyMMTKGlobal {
     .cond_world_stopped = PTHREAD_COND_INITIALIZER,
     .cond_world_started = PTHREAD_COND_INITIALIZER,
     .mutator_blocking_count = 0,
+    .gc_disabling_state = (struct RubyMMTKGCDisablingState) {
+        .mutex = PTHREAD_MUTEX_INITIALIZER,
+        .gc_is_disabled = false,
+    },
     .fork_hook_vm_lock_lev = 0,
     .world_stopped = false,
     .start_the_world_count = 0,
@@ -1802,7 +1814,83 @@ rb_mmtk_block_for_gc_internal(void *unused)
     RUBY_DEBUG_LOG("GC finished.");
 }
 
-static void
+////////////////////////////////////////////////////////////////////////////////
+// Disabling / enabling GC
+////////////////////////////////////////////////////////////////////////////////
+
+void
+rb_mmtk_disable_collection()
+{
+    // Only mutator threads can disable GC.
+    rb_mmtk_assert_mutator();
+
+    struct RubyMMTKGCDisablingState *state = &rb_mmtk_global.gc_disabling_state;
+
+    for (;;) {
+        pthread_mutex_lock(&state->mutex);
+        bool gc_is_disabled = state->gc_is_disabled;
+        if (!gc_is_disabled) {
+            // GC is not disabled, yet.  We try once to disable it.
+            bool successful = mmtk_disable_collection();
+            if (successful) {
+                // If it is successful, we have disabled GC.  We set the state value.
+                state->gc_is_disabled = true;
+                gc_is_disabled = true;
+            }
+        }
+        pthread_mutex_unlock(&state->mutex);
+
+        if (gc_is_disabled) {
+            // Either GC has already been disabled before this call,
+            // or we successfully disabled GC.  We can return.
+            return;
+        } else {
+            // Otherwise, another thread must have triggered GC, and GC will start soon.
+            // We block until the next GC finishes and try again.
+            rb_mmtk_block_for_gc((MMTk_VMMutatorThread)GET_RACTOR());
+        }
+    }
+}
+
+void
+rb_mmtk_enable_collection()
+{
+    // Only mutator threads can enable GC.
+    rb_mmtk_assert_mutator();
+
+    struct RubyMMTKGCDisablingState *state = &rb_mmtk_global.gc_disabling_state;
+
+    pthread_mutex_lock(&state->mutex);
+    bool gc_is_disabled = state->gc_is_disabled;
+    if (gc_is_disabled) {
+        // GC is already disabled.  We enable GC.  This is non-blocking.
+        mmtk_enable_collection();
+        state->gc_is_disabled = false;
+    }
+    pthread_mutex_unlock(&state->mutex);
+
+}
+
+bool
+rb_mmtk_is_collection_enabled()
+{
+    // Only mutator threads can enable GC.
+    rb_mmtk_assert_mutator();
+
+    struct RubyMMTKGCDisablingState *state = &rb_mmtk_global.gc_disabling_state;
+
+    pthread_mutex_lock(&state->mutex);
+    bool gc_is_disabled = state->gc_is_disabled;
+    pthread_mutex_unlock(&state->mutex);
+
+    return !gc_is_disabled;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Blocking for GC
+////////////////////////////////////////////////////////////////////////////////
+
+void
 rb_mmtk_block_for_gc(MMTk_VMMutatorThread tls)
 {
     rb_mmtk_assert_mutator();

test/.excludes-mmtk/TestGc.rb

@@ -4,6 +4,7 @@
 exclude(:test_gc_config_full_mark_by_default, "testing behaviour specific to default GC")
 exclude(:test_gc_config_invalid_args, "testing behaviour specific to default GC")
 exclude(:test_gc_config_setting_returns_updated_config_hash, "testing behaviour specific to default GC")
+exclude(:test_gc_disabled_start, "When GC is disabled on MMTk, GC cannot be triggered in any way.")
 exclude(:test_gc_internals, "testing behaviour specific to default GC")
 exclude(:test_gc_parameter, "testing behaviour specific to default GC")
 exclude(:test_gc_parameter_init_slots, "testing behaviour specific to default GC")

test/.excludes-mmtk/TestObjSpace.rb

@@ -4,3 +4,4 @@
 exclude(:test_memsize_of, "testing behaviour specific to default GC")
 exclude(:test_memsize_of_root_shared_string, "testing behaviour specific to default GC")
 exclude(:test_dump_objects_dumps_page_slot_sizes, "testing behaviour specific to default GC")
+

vm.c

@@ -4612,7 +4612,27 @@ Init_VM(void)
 #if USE_MMTK
         if (rb_mmtk_enabled_p()) {
             // Now that the VM says it's time to enable GC, we enable GC for MMTk, too.
+
+            // We don't use rb_bug just to be safe.  See Init_BareVM.
+
+            // Assert MMTk-level raw state.
+            if (mmtk_is_collection_enabled()) {
+                fprintf(stderr, "ERROR: GC is enabled at the MMTk level, but should still be disabled.\n");
+                abort();
+            }
+
+            // We enable GC at the MMTk level.
             mmtk_enable_collection();
+
+            // Assert both MMTk-level raw state and binding-level wrapped state.
+            if (!mmtk_is_collection_enabled()) {
+                fprintf(stderr, "ERROR: GC is disabled at the MMTk level, but should be enabled now.\n");
+                abort();
+            }
+            if (!rb_mmtk_is_collection_enabled()) {
+                fprintf(stderr, "ERROR: GC is disabled at the binding level, but should be enabled now.\n");
+                abort();
+            }
         }
 #endif
 
@@ -4674,7 +4694,34 @@ Init_BareVM(void)
         // When creating a ractor, it will bind mutator.
         // We pass NULL as the tls because `Collection::spawn_gc_thread` in the mmtk-ruby binding does not use it anyway.
         mmtk_initialize_collection(NULL);
-        // Note: GC is disabled at this moment.
+
+        // mmtk_initialize_collection() leaves collection enabled; explicitly disable it here so
+        // it stays off until Init_VM() re-enables it once the VM is fully bootstrapped. This is
+        // the only thread in the process at this point, and no GC has had a chance to run yet
+        // (nor could one be requested at this point in bootstrap), so this must always succeed
+        // immediately; if it doesn't, something is fundamentally broken, so abort rather than
+        // spin or silently continue with GC still enabled.
+        //
+        // Deliberately not rb_bug() here: rb_bug()'s crash-report path (rb_vm_bugreport()) is
+        // documented to be able to trigger a secondary SIGSEGV when walking frames on an
+        // abnormal VM state (see the comment in bug_report_file() in error.c), and at this point
+        // in bootstrap the main thread's execution context and ractor aren't set up yet -- so
+        // reporting via rb_bug() here risks turning a clean diagnostic into a mystery crash.
+
+        // Assert MMTk-level raw state.
+        if (!mmtk_is_collection_enabled()) {
+            fprintf(stderr, "ERROR: GC is disabled at the MMTk level, but should be enabled.\n");
+            abort();
+        }
+
+        // We disable GC at the MMTk level.
+        mmtk_disable_collection();
+
+        // Assert MMTk-level raw state.
+        if (mmtk_is_collection_enabled()) {
+            fprintf(stderr, "ERROR: GC is still enabled at the MMTk level, but should be disabled now.\n");
+            abort();
+        }
     })
 
     // setup main thread