Fix deadlock when malloc in Ractor lock

If we malloc when the current Ractor is locked, we can deadlock because
GC requires VM lock and Ractor barrier. If another Ractor is waiting on
this Ractor lock, then it will deadlock because the other Ractor will
never join the barrier.

For example, this script deadlocks:

    r = Ractor.new do
      loop do
        Ractor::Port.new
      end
    end

    100000.times do |i|
      r.send(nil)
      puts i
    end

On debug builds, it fails with this assertion error:

    vm_sync.c:75: Assertion Failed: vm_lock_enter:cr->sync.locked_by != rb_ractor_self(cr)

On non-debug builds, we can see that it deadlocks in the debugger:

    Main Ractor:
    frame #3: 0x000000010021fdc4 miniruby`rb_native_mutex_lock(lock=<unavailable>) at thread_pthread.c:115:14
    frame Shopify#4: 0x0000000100193eb8 miniruby`ractor_send0 [inlined] ractor_lock(r=<unavailable>, file=<unavailable>, line=1180) at ractor.c:73:5
    frame Shopify#5: 0x0000000100193eb0 miniruby`ractor_send0 [inlined] ractor_send_basket(ec=<unavailable>, rp=0x0000000131092840, b=0x000000011c63de80, raise_on_error=true) at ractor_sync.c:1180:5
    frame Shopify#6: 0x0000000100193eac miniruby`ractor_send0(ec=<unavailable>, rp=0x0000000131092840, obj=4, move=<unavailable>, raise_on_error=true) at ractor_sync.c:1211:5

    Second Ractor:
    frame #2: 0x00000001002208d0 miniruby`rb_ractor_sched_barrier_start [inlined] rb_native_cond_wait(cond=<unavailable>, mutex=<unavailable>) at thread_pthread.c:221:13
    frame #3: 0x00000001002208cc miniruby`rb_ractor_sched_barrier_start(vm=0x000000013180d600, cr=0x0000000131093460) at thread_pthread.c:1438:13
    frame Shopify#4: 0x000000010028a328 miniruby`rb_vm_barrier at vm_sync.c:262:13 [artificial]
    frame Shopify#5: 0x00000001000dfa6c miniruby`gc_start [inlined] rb_gc_vm_barrier at gc.c:179:5
    frame Shopify#6: 0x00000001000dfa68 miniruby`gc_start [inlined] gc_enter(objspace=0x000000013180fc00, event=gc_enter_event_start, lock_lev=<unavailable>) at default.c:6636:9
    frame Shopify#7: 0x00000001000dfa48 miniruby`gc_start(objspace=0x000000013180fc00, reason=<unavailable>) at default.c:6361:5
    frame Shopify#8: 0x00000001000e3fd8 miniruby`objspace_malloc_increase_body [inlined] garbage_collect(objspace=0x000000013180fc00, reason=512) at default.c:6341:15
    frame Shopify#9: 0x00000001000e3fa4 miniruby`objspace_malloc_increase_body [inlined] garbage_collect_with_gvl(objspace=0x000000013180fc00, reason=512) at default.c:6741:16
    frame Shopify#10: 0x00000001000e3f88 miniruby`objspace_malloc_increase_body(objspace=0x000000013180fc00, mem=<unavailable>, new_size=<unavailable>, old_size=<unavailable>, type=<unavailable>) at default.c:8007:13
    frame Shopify#11: 0x00000001000e3c44 miniruby`rb_gc_impl_malloc [inlined] objspace_malloc_fixup(objspace=0x000000013180fc00, mem=0x000000011c700000, size=12582912) at default.c:8085:5
    frame Shopify#12: 0x00000001000e3c30 miniruby`rb_gc_impl_malloc(objspace_ptr=0x000000013180fc00, size=12582912) at default.c:8182:12
    frame Shopify#13: 0x00000001000d4584 miniruby`ruby_xmalloc [inlined] ruby_xmalloc_body(size=<unavailable>) at gc.c:5128:12
    frame Shopify#14: 0x00000001000d4568 miniruby`ruby_xmalloc(size=<unavailable>) at gc.c:5118:34
    frame Shopify#15: 0x00000001001eb184 miniruby`rb_st_init_existing_table_with_size(tab=0x000000011c2b4b40, type=<unavailable>, size=<unavailable>) at st.c:559:39
    frame Shopify#16: 0x00000001001ebc74 miniruby`rebuild_table_if_necessary [inlined] rb_st_init_table_with_size(type=0x00000001004f4a78, size=524287) at st.c:585:5
    frame Shopify#17: 0x00000001001ebc5c miniruby`rebuild_table_if_necessary [inlined] rebuild_table(tab=0x000000013108e2f0) at st.c:753:19
    frame Shopify#18: 0x00000001001ebbfc miniruby`rebuild_table_if_necessary(tab=0x000000013108e2f0) at st.c:1125:9
    frame Shopify#19: 0x00000001001eba08 miniruby`rb_st_insert(tab=0x000000013108e2f0, key=262144, value=4767566624) at st.c:1143:5
    frame Shopify#20: 0x0000000100194b84 miniruby`ractor_port_initialzie [inlined] ractor_add_port(r=0x0000000131093460, id=262144) at ractor_sync.c:399:9
    frame Shopify#21: 0x0000000100194b58 miniruby`ractor_port_initialzie [inlined] ractor_port_init(rpv=4750065560, r=0x0000000131093460) at ractor_sync.c:87:5
    frame Shopify#22: 0x0000000100194b34 miniruby`ractor_port_initialzie(self=4750065560) at ractor_sync.c:103:12

Author: peterzhu2118

bootstraptest/test_ractor.rb

@@ -2372,3 +2372,26 @@ def call_test(obj)
   ractors.each(&:join)
   :ok
 RUBY
+
+# This test checks that we do not trigger a GC when we have malloc with Ractor
+# locks. We cannot trigger a GC with Ractor locks because GC requires VM lock
+# and Ractor barrier. If another Ractor is waiting on this Ractor lock, then it
+# will deadlock because the other Ractor will never join the barrier.
+#
+# Creating Ractor::Port requires locking the Ractor and inserting into an
+# st_table, which can call malloc.
+assert_equal 'ok', <<~'RUBY'
+  r = Ractor.new do
+    loop do
+      Ractor::Port.new
+    end
+  end
+
+  10.times do
+    10_000.times do
+      r.send(nil)
+    end
+    sleep(0.01)
+  end
+  :ok
+RUBY

gc.c

@@ -625,9 +625,9 @@ typedef struct gc_function_map {
     size_t (*heap_id_for_size)(void *objspace_ptr, size_t size);
     bool (*size_allocatable_p)(size_t size);
     // Malloc
-    void *(*malloc)(void *objspace_ptr, size_t size);
-    void *(*calloc)(void *objspace_ptr, size_t size);
-    void *(*realloc)(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size);
+    void *(*malloc)(void *objspace_ptr, size_t size, bool gc_allowed);
+    void *(*calloc)(void *objspace_ptr, size_t size, bool gc_allowed);
+    void *(*realloc)(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size, bool gc_allowed);
     void (*free)(void *objspace_ptr, void *ptr, size_t old_size);
     void (*adjust_memory_usage)(void *objspace_ptr, ssize_t diff);
     // Marking
@@ -5118,14 +5118,22 @@ ruby_xmalloc(size_t size)
     return handle_malloc_failure(ruby_xmalloc_body(size));
 }
 
+static bool
+malloc_gc_allowed(void)
+{
+    rb_ractor_t *r = rb_current_ractor_raw(false);
+
+    return r == NULL || !r->malloc_gc_disabled;
+}
+
 static void *
 ruby_xmalloc_body(size_t size)
 {
     if ((ssize_t)size < 0) {
         negative_size_allocation_error("too large allocation size");
     }
 
-    return rb_gc_impl_malloc(rb_gc_get_objspace(), size);
+    return rb_gc_impl_malloc(rb_gc_get_objspace(), size, malloc_gc_allowed());
 }
 
 void
@@ -5155,7 +5163,7 @@ ruby_xmalloc2(size_t n, size_t size)
 static void *
 ruby_xmalloc2_body(size_t n, size_t size)
 {
-    return rb_gc_impl_malloc(rb_gc_get_objspace(), xmalloc2_size(n, size));
+    return rb_gc_impl_malloc(rb_gc_get_objspace(), xmalloc2_size(n, size), malloc_gc_allowed());
 }
 
 static void *ruby_xcalloc_body(size_t n, size_t size);
@@ -5169,7 +5177,7 @@ ruby_xcalloc(size_t n, size_t size)
 static void *
 ruby_xcalloc_body(size_t n, size_t size)
 {
-    return rb_gc_impl_calloc(rb_gc_get_objspace(), xmalloc2_size(n, size));
+    return rb_gc_impl_calloc(rb_gc_get_objspace(), xmalloc2_size(n, size), malloc_gc_allowed());
 }
 
 static void *ruby_sized_xrealloc_body(void *ptr, size_t new_size, size_t old_size);
@@ -5190,7 +5198,7 @@ ruby_sized_xrealloc_body(void *ptr, size_t new_size, size_t old_size)
         negative_size_allocation_error("too large allocation size");
     }
 
-    return rb_gc_impl_realloc(rb_gc_get_objspace(), ptr, new_size, old_size);
+    return rb_gc_impl_realloc(rb_gc_get_objspace(), ptr, new_size, old_size, malloc_gc_allowed());
 }
 
 void *
@@ -5214,7 +5222,7 @@ static void *
 ruby_sized_xrealloc2_body(void *ptr, size_t n, size_t size, size_t old_n)
 {
     size_t len = xmalloc2_size(n, size);
-    return rb_gc_impl_realloc(rb_gc_get_objspace(), ptr, len, old_n * size);
+    return rb_gc_impl_realloc(rb_gc_get_objspace(), ptr, len, old_n * size, malloc_gc_allowed());
 }
 
 void *

gc/default/default.c

@@ -7970,7 +7970,7 @@ objspace_malloc_gc_stress(rb_objspace_t *objspace)
 }
 
 static inline bool
-objspace_malloc_increase_report(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type)
+objspace_malloc_increase_report(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type, bool gc_allowed)
 {
     if (0) fprintf(stderr, "increase - ptr: %p, type: %s, new_size: %"PRIdSIZE", old_size: %"PRIdSIZE"\n",
                    mem,
@@ -7982,7 +7982,7 @@ objspace_malloc_increase_report(rb_objspace_t *objspace, void *mem, size_t new_s
 }
 
 static bool
-objspace_malloc_increase_body(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type)
+objspace_malloc_increase_body(rb_objspace_t *objspace, void *mem, size_t new_size, size_t old_size, enum memop_type type, bool gc_allowed)
 {
     if (new_size > old_size) {
         RUBY_ATOMIC_SIZE_ADD(malloc_increase, new_size - old_size);
@@ -7997,7 +7997,7 @@ objspace_malloc_increase_body(rb_objspace_t *objspace, void *mem, size_t new_siz
 #endif
     }
 
-    if (type == MEMOP_TYPE_MALLOC) {
+    if (type == MEMOP_TYPE_MALLOC && gc_allowed) {
       retry:
         if (malloc_increase > malloc_limit && ruby_native_thread_p() && !dont_gc_val()) {
             if (ruby_thread_has_gvl_p() && is_lazy_sweeping(objspace)) {
@@ -8079,10 +8079,10 @@ malloc_during_gc_p(rb_objspace_t *objspace)
 }
 
 static inline void *
-objspace_malloc_fixup(rb_objspace_t *objspace, void *mem, size_t size)
+objspace_malloc_fixup(rb_objspace_t *objspace, void *mem, size_t size, bool gc_allowed)
 {
     size = objspace_malloc_size(objspace, mem, size);
-    objspace_malloc_increase(objspace, mem, size, 0, MEMOP_TYPE_MALLOC) {}
+    objspace_malloc_increase(objspace, mem, size, 0, MEMOP_TYPE_MALLOC, gc_allowed) {}
 
 #if CALC_EXACT_MALLOC_SIZE
     {
@@ -8114,10 +8114,10 @@ objspace_malloc_fixup(rb_objspace_t *objspace, void *mem, size_t size)
             GPR_FLAG_MALLOC;                                 \
         objspace_malloc_gc_stress(objspace);                 \
                                                              \
-        if (RB_LIKELY((expr))) {                                \
+        if (RB_LIKELY((expr))) {                             \
             /* Success on 1st try */                         \
         }                                                    \
-        else if (!garbage_collect_with_gvl(objspace, gpr)) { \
+        else if (gc_allowed && !garbage_collect_with_gvl(objspace, gpr)) { \
             /* @shyouhei thinks this doesn't happen */       \
             GC_MEMERROR("TRY_WITH_GC: could not GC");        \
         }                                                    \
@@ -8160,15 +8160,15 @@ rb_gc_impl_free(void *objspace_ptr, void *ptr, size_t old_size)
 #endif
     old_size = objspace_malloc_size(objspace, ptr, old_size);
 
-    objspace_malloc_increase(objspace, ptr, 0, old_size, MEMOP_TYPE_FREE) {
+    objspace_malloc_increase(objspace, ptr, 0, old_size, MEMOP_TYPE_FREE, true) {
         free(ptr);
         ptr = NULL;
         RB_DEBUG_COUNTER_INC(heap_xfree);
     }
 }
 
 void *
-rb_gc_impl_malloc(void *objspace_ptr, size_t size)
+rb_gc_impl_malloc(void *objspace_ptr, size_t size, bool gc_allowed)
 {
     rb_objspace_t *objspace = objspace_ptr;
     check_malloc_not_in_gc(objspace, "malloc");
@@ -8179,11 +8179,11 @@ rb_gc_impl_malloc(void *objspace_ptr, size_t size)
     TRY_WITH_GC(size, mem = malloc(size));
     RB_DEBUG_COUNTER_INC(heap_xmalloc);
     if (!mem) return mem;
-    return objspace_malloc_fixup(objspace, mem, size);
+    return objspace_malloc_fixup(objspace, mem, size, gc_allowed);
 }
 
 void *
-rb_gc_impl_calloc(void *objspace_ptr, size_t size)
+rb_gc_impl_calloc(void *objspace_ptr, size_t size, bool gc_allowed)
 {
     rb_objspace_t *objspace = objspace_ptr;
 
@@ -8199,27 +8199,27 @@ rb_gc_impl_calloc(void *objspace_ptr, size_t size)
     size = objspace_malloc_prepare(objspace, size);
     TRY_WITH_GC(size, mem = calloc1(size));
     if (!mem) return mem;
-    return objspace_malloc_fixup(objspace, mem, size);
+    return objspace_malloc_fixup(objspace, mem, size, gc_allowed);
 }
 
 void *
-rb_gc_impl_realloc(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size)
+rb_gc_impl_realloc(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size, bool gc_allowed)
 {
     rb_objspace_t *objspace = objspace_ptr;
 
     check_malloc_not_in_gc(objspace, "realloc");
 
     void *mem;
 
-    if (!ptr) return rb_gc_impl_malloc(objspace, new_size);
+    if (!ptr) return rb_gc_impl_malloc(objspace, new_size, gc_allowed);
 
     /*
      * The behavior of realloc(ptr, 0) is implementation defined.
      * Therefore we don't use realloc(ptr, 0) for portability reason.
      * see http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_400.htm
      */
     if (new_size == 0) {
-        if ((mem = rb_gc_impl_malloc(objspace, 0)) != NULL) {
+        if ((mem = rb_gc_impl_malloc(objspace, 0, gc_allowed)) != NULL) {
             /*
              * - OpenBSD's malloc(3) man page says that when 0 is passed, it
              *   returns a non-NULL pointer to an access-protected memory page.
@@ -8278,7 +8278,7 @@ rb_gc_impl_realloc(void *objspace_ptr, void *ptr, size_t new_size, size_t old_si
     }
 #endif
 
-    objspace_malloc_increase(objspace, mem, new_size, old_size, MEMOP_TYPE_REALLOC);
+    objspace_malloc_increase(objspace, mem, new_size, old_size, MEMOP_TYPE_REALLOC, gc_allowed);
 
     RB_DEBUG_COUNTER_INC(heap_xrealloc);
     return mem;
@@ -8290,10 +8290,10 @@ rb_gc_impl_adjust_memory_usage(void *objspace_ptr, ssize_t diff)
     rb_objspace_t *objspace = objspace_ptr;
 
     if (diff > 0) {
-        objspace_malloc_increase(objspace, 0, diff, 0, MEMOP_TYPE_REALLOC);
+        objspace_malloc_increase(objspace, 0, diff, 0, MEMOP_TYPE_REALLOC, true);
     }
     else if (diff < 0) {
-        objspace_malloc_increase(objspace, 0, 0, -diff, MEMOP_TYPE_REALLOC);
+        objspace_malloc_increase(objspace, 0, 0, -diff, MEMOP_TYPE_REALLOC, true);
     }
 }
 

gc/gc_impl.h

@@ -72,9 +72,9 @@ GC_IMPL_FN bool rb_gc_impl_size_allocatable_p(size_t size);
  * memory just return NULL (with appropriate errno set).
  * The caller side takes care of that situation.
  */
-GC_IMPL_FN void *rb_gc_impl_malloc(void *objspace_ptr, size_t size);
-GC_IMPL_FN void *rb_gc_impl_calloc(void *objspace_ptr, size_t size);
-GC_IMPL_FN void *rb_gc_impl_realloc(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size);
+GC_IMPL_FN void *rb_gc_impl_malloc(void *objspace_ptr, size_t size, bool gc_allowed);
+GC_IMPL_FN void *rb_gc_impl_calloc(void *objspace_ptr, size_t size, bool gc_allowed);
+GC_IMPL_FN void *rb_gc_impl_realloc(void *objspace_ptr, void *ptr, size_t new_size, size_t old_size, bool gc_allowed);
 GC_IMPL_FN void rb_gc_impl_free(void *objspace_ptr, void *ptr, size_t old_size);
 GC_IMPL_FN void rb_gc_impl_adjust_memory_usage(void *objspace_ptr, ssize_t diff);
 // Marking

ractor.c

@@ -71,6 +71,7 @@ ractor_lock(rb_ractor_t *r, const char *file, int line)
 
     ASSERT_ractor_unlocking(r);
     rb_native_mutex_lock(&r->sync.lock);
+    r->malloc_gc_disabled = true;
 
 #if RACTOR_CHECK_MODE > 0
     if (rb_current_execution_context(false) != NULL) {
@@ -99,6 +100,10 @@ ractor_unlock(rb_ractor_t *r, const char *file, int line)
 #if RACTOR_CHECK_MODE > 0
     r->sync.locked_by = Qnil;
 #endif
+
+    VM_ASSERT(r->malloc_gc_disabled);
+
+    r->malloc_gc_disabled = false;
     rb_native_mutex_unlock(&r->sync.lock);
 
     RUBY_DEBUG_LOG2(file, line, "r:%u%s", r->pub.id, rb_current_ractor_raw(false) == r ? " (self)" : "");

ractor_core.h

@@ -98,6 +98,7 @@ struct rb_ractor_struct {
     VALUE verbose;
     VALUE debug;
 
+    bool malloc_gc_disabled;
     void *newobj_cache;
 }; // rb_ractor_t is defined in vm_core.h
 

ractor_sync.c

@@ -395,7 +395,6 @@ ractor_add_port(rb_ractor_t *r, st_data_t id)
 
     RACTOR_LOCK(r);
     {
-        // memo: can cause GC, but GC doesn't use ractor locking.
         st_insert(r->sync.ports, id, (st_data_t)rq);
     }
     RACTOR_UNLOCK(r);