[memory_view] Make MemoryView API Ractor-safe (#3911)

* memory_view.c: make Ractor-safe

* test/ruby/test_memory_view.rb: Add test_ractor

* memory_view: fix typo

* memory_view.c: Use st_update in unregster_exported_object

* memory_view: update dependency

common.mk

@@ -8018,6 +8018,7 @@ memory_view.$(OBJEXT): {$(VPATH)}backward/2/stdalign.h
 memory_view.$(OBJEXT): {$(VPATH)}backward/2/stdarg.h
 memory_view.$(OBJEXT): {$(VPATH)}config.h
 memory_view.$(OBJEXT): {$(VPATH)}constant.h
+memory_view.$(OBJEXT): {$(VPATH)}debug_counter.h
 memory_view.$(OBJEXT): {$(VPATH)}defines.h
 memory_view.$(OBJEXT): {$(VPATH)}id_table.h
 memory_view.$(OBJEXT): {$(VPATH)}intern.h
@@ -8165,8 +8166,11 @@ memory_view.$(OBJEXT): {$(VPATH)}internal/xmalloc.h
 memory_view.$(OBJEXT): {$(VPATH)}memory_view.c
 memory_view.$(OBJEXT): {$(VPATH)}memory_view.h
 memory_view.$(OBJEXT): {$(VPATH)}missing.h
+memory_view.$(OBJEXT): {$(VPATH)}node.h
 memory_view.$(OBJEXT): {$(VPATH)}st.h
 memory_view.$(OBJEXT): {$(VPATH)}subst.h
+memory_view.$(OBJEXT): {$(VPATH)}vm_debug.h
+memory_view.$(OBJEXT): {$(VPATH)}vm_sync.h
 miniinit.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h
 miniinit.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h
 miniinit.$(OBJEXT): $(CCAN_DIR)/list/list.h

ext/-test-/memory_view/memory_view.c

@@ -370,6 +370,7 @@ mdview_aref(VALUE obj, VALUE indices_v)
 void
 Init_memory_view(void)
 {
+    rb_ext_ractor_safe(true);
     VALUE mMemoryViewTestUtils = rb_define_module("MemoryViewTestUtils");
 
     rb_define_module_function(mMemoryViewTestUtils, "available?", memory_view_available_p, 1);

memory_view.c

@@ -11,6 +11,7 @@
 #include "internal/variable.h"
 #include "internal/util.h"
 #include "ruby/memory_view.h"
+#include "vm_sync.h"
 
 #if SIZEOF_INTPTR_T == SIZEOF_LONG_LONG
 #   define INTPTR2NUM LL2NUM
@@ -30,6 +31,7 @@
 
 // Exported Object Registry
 
+static st_table *exported_object_table = NULL;
 VALUE rb_memory_view_exported_object_registry = Qundef;
 
 static int
@@ -42,16 +44,18 @@ exported_object_registry_mark_key_i(st_data_t key, st_data_t value, st_data_t da
 static void
 exported_object_registry_mark(void *ptr)
 {
-    st_table *table = ptr;
-    st_foreach(table, exported_object_registry_mark_key_i, 0);
+    // Don't use RB_VM_LOCK_ENTER here.  It is unnecessary during GC.
+    st_foreach(exported_object_table, exported_object_registry_mark_key_i, 0);
 }
 
 static void
 exported_object_registry_free(void *ptr)
 {
-    st_table *table = ptr;
-    st_clear(table);
-    st_free_table(table);
+    // Note that calling RB_VM_LOCK_ENTER here is unnecessary now.
+    // But it may be changed in the future.
+    st_clear(exported_object_table);
+    st_free_table(exported_object_table);
+    exported_object_table = NULL;
 }
 
 const rb_data_type_t rb_memory_view_exported_object_registry_data_type = {
@@ -64,33 +68,11 @@ const rb_data_type_t rb_memory_view_exported_object_registry_data_type = {
     0, 0, RUBY_TYPED_FREE_IMMEDIATELY
 };
 
-static void
-init_exported_object_registry(void)
-{
-    if (rb_memory_view_exported_object_registry != Qundef) {
-        return;
-    }
-
-    st_table *table = rb_init_identtable();
-    VALUE obj = TypedData_Wrap_Struct(
-        0, &rb_memory_view_exported_object_registry_data_type, table);
-    rb_gc_register_mark_object(obj);
-    rb_memory_view_exported_object_registry = obj;
-}
-
-static inline st_table *
-get_exported_object_table(void)
-{
-    st_table *table;
-    TypedData_Get_Struct(rb_memory_view_exported_object_registry, st_table,
-                         &rb_memory_view_exported_object_registry_data_type,
-                         table);
-    return table;
-}
-
 static int
-update_exported_object_ref_count(st_data_t *key, st_data_t *val, st_data_t arg, int existing)
+exported_object_add_ref(st_data_t *key, st_data_t *val, st_data_t arg, int existing)
 {
+    ASSERT_vm_locking();
+
     if (existing) {
         *val += 1;
     }
@@ -100,39 +82,34 @@ update_exported_object_ref_count(st_data_t *key, st_data_t *val, st_data_t arg,
     return ST_CONTINUE;
 }
 
+static int
+exported_object_dec_ref(st_data_t *key, st_data_t *val, st_data_t arg, int existing)
+{
+    ASSERT_vm_locking();
+
+    if (existing) {
+        *val -= 1;
+        if (*val == 0) {
+            return ST_DELETE;
+        }
+    }
+    return ST_CONTINUE;
+}
+
 static void
 register_exported_object(VALUE obj)
 {
-    if (rb_memory_view_exported_object_registry == Qundef) {
-        init_exported_object_registry();
-    }
-
-    st_table *table = get_exported_object_table();
-
-    st_update(table, (st_data_t)obj, update_exported_object_ref_count, 0);
+    RB_VM_LOCK_ENTER();
+    st_update(exported_object_table, (st_data_t)obj, exported_object_add_ref, 0);
+    RB_VM_LOCK_LEAVE();
 }
 
 static void
 unregister_exported_object(VALUE obj)
 {
-    if (rb_memory_view_exported_object_registry == Qundef) {
-        return;
-    }
-
-    st_table *table = get_exported_object_table();
-
-    st_data_t count;
-    if (!st_lookup(table, (st_data_t)obj, &count)) {
-        return;
-    }
-
-    if (--count == 0) {
-        st_data_t key = (st_data_t)obj;
-        st_delete(table, &key, &count);
-    }
-    else {
-        st_insert(table, (st_data_t)obj, count);
-    }
+    RB_VM_LOCK_ENTER();
+    st_update(exported_object_table, (st_data_t)obj, exported_object_dec_ref, 0);
+    RB_VM_LOCK_LEAVE();
 }
 
 // MemoryView
@@ -876,5 +853,15 @@ rb_memory_view_release(rb_memory_view_t* view)
 void
 Init_MemoryView(void)
 {
+    exported_object_table = rb_init_identtable();
+
+    // exported_object_table is referred through rb_memory_view_exported_object_registry
+    // in -test-/memory_view extension.
+    VALUE obj = TypedData_Wrap_Struct(
+        0, &rb_memory_view_exported_object_registry_data_type,
+        exported_object_table);
+    rb_gc_register_mark_object(obj);
+    rb_memory_view_exported_object_registry = obj;
+
     id_memory_view = rb_intern_const("__memory_view__");
 }

test/ruby/test_memory_view.rb

@@ -320,4 +320,22 @@ class TestMemoryView < Test::Unit::TestCase
     assert_equal([-1, -2], mv[[1, 0]])
     assert_equal([-7, -8], mv[[1, 3]])
   end
+
+  def test_ractor
+    assert_in_out_err([], <<-"end;", ["[5, 6]", "[-7, -8]"], [])
+      require "-test-/memory_view"
+      require "rbconfig/sizeof"
+      $VERBOSE = nil
+      r = Ractor.new RbConfig::SIZEOF["short"] do |sizeof_short|
+        buf = [ 1, 2,  3,  4,  5,  6,  7,  8,
+                -1, -2, -3, -4, -5, -6, -7, -8].pack("s*")
+        shape = [2, 4]
+        strides = [4*sizeof_short*2, sizeof_short*2]
+        mv = MemoryViewTestUtils::MultiDimensionalView.new(buf, "ss", shape, strides)
+        p mv[[0, 2]]
+        mv[[1, 3]]
+      end
+      p r.take
+    end;
+  end
 end