Reintorduce gc_disable/enable API

benches/mock_bench/alloc.rs

@@ -2,23 +2,12 @@ use criterion::Criterion;
 
 use mmtk::memory_manager;
 use mmtk::util::test_util::fixtures::*;
-use mmtk::util::test_util::mock_method::*;
-use mmtk::util::test_util::mock_vm::{write_mockvm, MockVM};
 use mmtk::AllocationSemantics;
 
 pub fn bench(c: &mut Criterion) {
-    // Setting a larger heap, although the GC should be disabled in the MockVM
+    // Setting a larger heap, although the GC should be disabled below
     let mut fixture = MutatorFixture::create_with_heapsize(1 << 30);
-    {
-        write_mockvm(|mock| {
-            *mock = {
-                MockVM {
-                    is_collection_enabled: MockMethod::new_fixed(Box::new(|_| false)),
-                    ..MockVM::default()
-                }
-            }
-        });
-    }
+    memory_manager::disable_collection(fixture.mmtk());
 
     c.bench_function("alloc", |b| {
         b.iter(|| {

benches/mock_bench/internal_pointer.rs

@@ -1,22 +1,25 @@
 use criterion::Criterion;
 
+#[cfg(feature = "vo_bit")]
+use mmtk::memory_manager;
 #[cfg(feature = "vo_bit")]
 use mmtk::util::test_util::fixtures::*;
 use mmtk::util::test_util::mock_method::*;
 use mmtk::util::test_util::mock_vm::{write_mockvm, MockVM};
 
 pub fn bench(c: &mut Criterion) {
-    // Setting a larger heap, although the GC should be disabled in the MockVM
+    // Setting a larger heap, although the GC should be disabled below
     #[cfg(feature = "vo_bit")]
     let mut fixture = MutatorFixture::create_with_heapsize(1 << 30);
+    #[cfg(feature = "vo_bit")]
+    memory_manager::disable_collection(fixture.mmtk());
 
     // Normal objects
     // 16KB object -- we want to make sure the object can fit into any normal space (e.g. immix space or mark sweep space)
     const NORMAL_OBJECT_SIZE: usize = 16 * 1024;
     write_mockvm(|mock| {
         *mock = MockVM {
             get_object_size: MockMethod::new_fixed(Box::new(|_| NORMAL_OBJECT_SIZE)),
-            is_collection_enabled: MockMethod::new_fixed(Box::new(|_| false)),
             ..MockVM::default()
         }
     });
@@ -55,7 +58,6 @@ pub fn bench(c: &mut Criterion) {
     write_mockvm(|mock| {
         *mock = MockVM {
             get_object_size: MockMethod::new_fixed(Box::new(|_| LARGE_OBJECT_SIZE)),
-            is_collection_enabled: MockMethod::new_fixed(Box::new(|_| false)),
             ..MockVM::default()
         }
     });

src/memory_manager.rs

@@ -577,6 +577,26 @@ pub fn initialize_collection<VM: VMBinding>(mmtk: &'static MMTK<VM>, tls: VMThre
     mmtk.initialize_collection(tls);
 }
 
+/// Wrapper for [`crate::mmtk::MMTK::disable_collection`].
+pub fn disable_collection<VM: VMBinding>(mmtk: &MMTK<VM>) {
+    mmtk.disable_collection();
+}
+
+/// Wrapper for [`crate::mmtk::MMTK::enable_collection`].
+pub fn enable_collection<VM: VMBinding>(mmtk: &MMTK<VM>) {
+    mmtk.enable_collection();
+}
+
+/// Wrapper for [`crate::mmtk::MMTK::is_collection_enabled`].
+pub fn is_collection_enabled<VM: VMBinding>(mmtk: &MMTK<VM>) -> bool {
+    mmtk.is_collection_enabled()
+}
+
+/// Wrapper for [`crate::mmtk::MMTK::wait_for_no_collection_in_progress`].
+pub fn wait_for_no_collection_in_progress<VM: VMBinding>(mmtk: &MMTK<VM>) {
+    mmtk.wait_for_no_collection_in_progress();
+}
+
 /// Process MMTk run-time options. Returns true if the option is processed successfully.
 ///
 /// Arguments:

src/mmtk.rs

@@ -399,6 +399,49 @@ impl<VM: VMBinding> MMTK<VM> {
         *self.state.gc_status.lock().unwrap() == GcStatus::GcProper
     }
 
+    /// Disable collection. MMTk will not trigger a GC while collection is disabled, though a GC
+    /// that has already been requested (or is already under way, including the concurrent phase
+    /// of a concurrent GC) is not aborted by this call. Use
+    /// [`MMTK::wait_for_no_collection_in_progress`] to wait for such a GC to fully finish.
+    ///
+    /// This call is nestable. Each call must be paired with a matching call to
+    /// [`MMTK::enable_collection`].
+    pub fn disable_collection(&self) {
+        self.gc_trigger.disable_collection();
+    }
+
+    /// Re-enable collection. Calling it without a prior matching call to
+    /// [`MMTK::disable_collection`] is a logic error.
+    pub fn enable_collection(&self) {
+        self.gc_trigger.enable_collection();
+    }
+
+    /// Return whether collection is currently enabled.
+    pub fn is_collection_enabled(&self) -> bool {
+        self.gc_trigger.is_collection_enabled()
+    }
+
+    /// Block the calling thread until there is no collection currently pending or in progress,
+    /// including the concurrent marking phase of a concurrent GC (during which mutators run
+    /// normally rather than being stopped). This does not itself prevent a new collection from
+    /// starting immediately afterwards; call [`MMTK::disable_collection`] first if that must
+    /// also be prevented.
+    ///
+    /// This is intended for infrequent, safety-sensitive call sites (e.g. adopting a foreign
+    /// thread, or recovering from a fatal signal) rather than as a general-purpose GC barrier,
+    /// so it is implemented as a simple poll loop (yielding the calling thread between checks)
+    /// rather than a condition variable.
+    pub fn wait_for_no_collection_in_progress(&self) {
+        let concurrent_work_in_progress = || {
+            self.get_plan()
+                .concurrent()
+                .is_some_and(|c| c.concurrent_work_in_progress())
+        };
+        while self.gc_trigger.has_pending_or_active_pause() || concurrent_work_in_progress() {
+            std::thread::yield_now();
+        }
+    }
+
     /// Return true if the current GC is an emergency GC.
     ///
     /// An emergency GC happens when a normal GC cannot reclaim enough memory to satisfy allocation

src/util/heap/gc_trigger.rs

@@ -1,18 +1,17 @@
 use atomic::Ordering;
 
-use crate::global_state::GlobalState;
+use crate::global_state::{GcStatus, GlobalState};
 use crate::plan::Plan;
 use crate::policy::space::Space;
 use crate::scheduler::GCWorkScheduler;
 use crate::util::constants::BYTES_IN_PAGE;
 use crate::util::conversions;
 use crate::util::options::{GCTriggerSelector, Options, DEFAULT_MAX_NURSERY, DEFAULT_MIN_NURSERY};
-use crate::vm::Collection;
 use crate::vm::VMBinding;
 use crate::MMTK;
 use std::mem::MaybeUninit;
 use std::sync::atomic::{AtomicBool, AtomicUsize};
-use std::sync::Arc;
+use std::sync::{Arc, Mutex};
 
 /// GCTrigger is responsible for triggering GCs based on the given policy.
 /// All the decisions about heap limit and GC triggering should be resolved here.
@@ -27,6 +26,12 @@ pub struct GCTrigger<VM: VMBinding> {
     /// Set by mutators to trigger GC.  It is atomic so that mutators can check if GC has already
     /// been requested efficiently in `poll` without acquiring any mutex.
     request_flag: AtomicBool,
+    /// Nesting depth for collection-disable scopes requested via [`GCTrigger::disable_collection`].
+    /// A depth of 0 means collection is enabled.
+    collection_disable_depth: AtomicUsize,
+    /// Serializes [`GCTrigger::disable_collection`]/[`GCTrigger::enable_collection`] with the
+    /// decision to request a GC, so that a GC is never requested while collection is disabled.
+    collection_control_lock: Mutex<()>,
     scheduler: Arc<GCWorkScheduler<VM>>,
     options: Arc<Options>,
     state: Arc<GlobalState>,
@@ -63,6 +68,8 @@ impl<VM: VMBinding> GCTrigger<VM> {
             },
             options,
             request_flag: AtomicBool::new(false),
+            collection_disable_depth: AtomicUsize::new(0),
+            collection_control_lock: Mutex::new(()),
             scheduler,
             state,
         }
@@ -79,9 +86,19 @@ impl<VM: VMBinding> GCTrigger<VM> {
 
     /// Request a GC.  Called by mutators when polling (during allocation) and when handling user
     /// GC requests (e.g. `System.gc();` in Java).
-    fn request(&self) {
+    /// Atomically check that collection is enabled, and if so, request a GC. This makes the
+    /// enabled-check and the request atomic with respect to [`GCTrigger::disable_collection`]
+    /// and [`GCTrigger::enable_collection`], so a GC is never requested after collection has
+    /// been disabled.
+    /// Returns whether a GC was actually requested.
+    fn request(&self) -> bool {
+        let _guard = self.collection_control_lock.lock().unwrap();
+        if self.collection_disable_depth.load(Ordering::Acquire) > 0 {
+            return false;
+        }
+
         if self.request_flag.load(Ordering::Relaxed) {
-            return;
+            return true;
         }
 
         if !self.request_flag.swap(true, Ordering::Relaxed) {
@@ -91,6 +108,8 @@ impl<VM: VMBinding> GCTrigger<VM> {
             probe!(mmtk, gc_requested);
             self.scheduler.request_schedule_collection();
         }
+
+        true
     }
 
     /// Clear the "GC requested" flag so that mutators can trigger the next GC.
@@ -99,6 +118,46 @@ impl<VM: VMBinding> GCTrigger<VM> {
         self.request_flag.store(false, Ordering::Relaxed);
     }
 
+    /// Disable collection. MMTk will not trigger a GC while collection is disabled, though a GC
+    /// that has already been requested (or is already under way, including the concurrent phase
+    /// of a concurrent GC) is not aborted by this call. Use
+    /// [`crate::MMTK::wait_for_no_collection_in_progress`] to wait for such a GC to fully finish.
+    ///
+    /// This call is nestable. Each call must be paired with a matching call to
+    /// [`GCTrigger::enable_collection`].
+    pub fn disable_collection(&self) {
+        let _guard = self.collection_control_lock.lock().unwrap();
+        self.collection_disable_depth
+            .fetch_add(1, Ordering::Release);
+    }
+
+    /// Re-enable collection. Calling it without a prior matching call to
+    /// [`GCTrigger::disable_collection`] is a logic error.
+    pub fn enable_collection(&self) {
+        let _guard = self.collection_control_lock.lock().unwrap();
+        let depth = self.collection_disable_depth.load(Ordering::Acquire);
+        assert!(
+            depth > 0,
+            "enable_collection() called without a matching disable_collection()"
+        );
+        self.collection_disable_depth
+            .store(depth - 1, Ordering::Release);
+    }
+
+    /// Return whether collection is currently enabled.
+    pub fn is_collection_enabled(&self) -> bool {
+        self.collection_disable_depth.load(Ordering::Acquire) == 0
+    }
+
+    /// Return whether a GC has been requested but not yet started (i.e. mutators have not all
+    /// stopped), or a stop-the-world pause is currently active. This does not account for the
+    /// concurrent marking phase of a concurrent GC, which mutators run through normally; see
+    /// `ConcurrentPlan::concurrent_work_in_progress`.
+    pub(crate) fn has_pending_or_active_pause(&self) -> bool {
+        self.request_flag.load(Ordering::Acquire)
+            || *self.state.gc_status.lock().unwrap() != GcStatus::NotInGC
+    }
+
     /// This method is called periodically by the allocation subsystem
     /// (by default, each time a page is consumed), and provides the
     /// collector with an opportunity to collect.
@@ -107,7 +166,7 @@ impl<VM: VMBinding> GCTrigger<VM> {
     /// * `space_full`: Space request failed, must recover pages within 'space'.
     /// * `space`: The space that triggered the poll. This could `None` if the poll is not triggered by a space.
     pub fn poll(&self, space_full: bool, space: Option<&dyn Space<VM>>) -> bool {
-        if !VM::VMCollection::is_collection_enabled() {
+        if !self.is_collection_enabled() {
             return false;
         }
 
@@ -127,8 +186,7 @@ impl<VM: VMBinding> GCTrigger<VM> {
                 plan.get_reserved_pages(),
                 plan.get_total_pages(),
             );
-            self.request();
-            return true;
+            return self.request();
         }
         false
     }
@@ -145,7 +203,7 @@ impl<VM: VMBinding> GCTrigger<VM> {
             return false;
         }
 
-        if force || !*self.options.ignore_system_gc && VM::VMCollection::is_collection_enabled() {
+        if force || !*self.options.ignore_system_gc && self.is_collection_enabled() {
             info!("User triggering collection");
             // TODO: this may not work reliably. If a GC has been triggered, this will not force it to be a full heap GC.
             if exhaustive {
@@ -157,8 +215,7 @@ impl<VM: VMBinding> GCTrigger<VM> {
             self.state
                 .user_triggered_collection
                 .store(true, Ordering::Relaxed);
-            self.request();
-            return true;
+            return self.request();
         }
 
         false

src/util/test_util/mock_vm.rs

@@ -215,7 +215,6 @@ pub struct MockVM {
     pub schedule_finalization: MockMethod<VMWorkerThread, ()>,
     pub post_forwarding: MockMethod<VMWorkerThread, ()>,
     pub vm_live_bytes: MockMethod<(), usize>,
-    pub is_collection_enabled: MockMethod<(), bool>,
     pub create_gc_trigger: MockMethod<(), Box<dyn GCTriggerPolicy<MockVM>>>,
     // object model
     pub copy_object: MockMethod<
@@ -290,7 +289,6 @@ impl Default for MockVM {
             schedule_finalization: MockMethod::new_default(),
             post_forwarding: MockMethod::new_default(),
             vm_live_bytes: MockMethod::new_default(),
-            is_collection_enabled: MockMethod::new_fixed(Box::new(|_| true)),
             create_gc_trigger: MockMethod::new_unimplemented(),
 
             copy_object: MockMethod::new_unimplemented(),
@@ -455,10 +453,6 @@ impl crate::vm::Collection<MockVM> for MockVM {
         mock!(post_forwarding(tls))
     }
 
-    fn is_collection_enabled() -> bool {
-        mock!(is_collection_enabled())
-    }
-
     fn vm_live_bytes() -> usize {
         mock!(vm_live_bytes())
     }

src/vm/collection.rs

@@ -179,25 +179,6 @@ pub trait Collection<VM: VMBinding> {
         0
     }
 
-    /// Callback function to ask the VM whether GC is enabled or disabled, allowing or disallowing MMTk
-    /// to trigger garbage collection. When collection is disabled, you can still allocate through MMTk,
-    /// but MMTk will not trigger a GC even if the heap is full. In such a case, the allocation will
-    /// exceed MMTk's heap size (the soft heap limit). However, there is no guarantee that the physical
-    /// allocation will succeed, and if it succeeds, there is no guarantee that further allocation will
-    /// keep succeeding. So if a VM disables collection, it needs to allocate with careful consideration
-    /// to make sure that the physical memory allows the amount of allocation. We highly recommend
-    /// to have GC always enabled (i.e. that this method always returns true). However, we support
-    /// this to accomodate some VMs that require this behavior. Note that
-    /// `handle_user_collection_request()` calls this function, too.  If this function returns
-    /// false, `handle_user_collection_request()` will not trigger GC, either. Note also that any synchronization
-    /// involving enabling and disabling collections by mutator threads should be implemented by the VM.
-    fn is_collection_enabled() -> bool {
-        // By default, MMTk assumes that collections are always enabled, and the binding should define
-        // this method if the VM supports disabling GC, or if the VM cannot safely trigger GC until some
-        // initialization is done, such as initializing class metadata for scanning objects.
-        true
-    }
-
     /// Ask the binding to create a [`GCTriggerPolicy`] if the option `gc_trigger` is set to
     /// `crate::util::options::GCTriggerSelector::Delegated`.
     fn create_gc_trigger() -> Box<dyn GCTriggerPolicy<VM>> {

src/vm/tests/mock_tests/mock_test_allocate_nonmoving.rs

@@ -6,12 +6,7 @@ use crate::plan::AllocationSemantics;
 #[test]
 pub fn allocate_nonmoving() {
     with_mockvm(
-        || -> MockVM {
-            MockVM {
-                is_collection_enabled: MockMethod::new_fixed(Box::new(|_| false)),
-                ..MockVM::default()
-            }
-        },
+        MockVM::default,
         || {
             // 1MB heap
             const MB: usize = 1024 * 1024;

src/vm/tests/mock_tests/mock_test_allocate_with_disable_collection.rs

@@ -1,24 +1,19 @@
 use crate::memory_manager;
 use crate::util::test_util::fixtures::*;
 use crate::util::test_util::mock_vm::*;
-use crate::vm::tests::mock_tests::mock_test_prelude::MockMethod;
 use crate::AllocationSemantics;
 
 /// This test allocates after calling disable_collection(). When we exceed the heap limit, MMTk will NOT trigger a GC.
 /// And the allocation will succeed.
 #[test]
 pub fn allocate_with_disable_collection() {
     with_mockvm(
-        || -> MockVM {
-            MockVM {
-                is_collection_enabled: MockMethod::new_fixed(Box::new(|_| false)),
-                ..MockVM::default()
-            }
-        },
+        MockVM::default,
         || {
             // 1MB heap
             const MB: usize = 1024 * 1024;
             let mut fixture = MutatorFixture::create_with_heapsize(MB);
+            memory_manager::disable_collection(fixture.mmtk());
 
             // Allocate half MB. It should be fine.
             let addr = memory_manager::alloc(