Revert "ns_subclasses refcount accesses need to be atomic (#15083)" (#15138)

luke-gruber · web-flow · commit 148fde27545e · 2025-11-11T02:52:43.000Z
This reverts commit 2998c8d. We need to find a better way to fix this bug. Even with this refcount change, errors were still being seen in CI. For now we need to remove this failing test.
diff --git a/class.c b/class.c
@@ -671,7 +671,7 @@ rb_class_classext_free_subclasses(rb_classext_t *ext, VALUE klass, bool replacin
     }
     VM_ASSERT(
         rb_box_subclasses_ref_count(anchor->box_subclasses) > 0,
-        "box_subclasses refcount (%p) %d", anchor->box_subclasses, rb_box_subclasses_ref_count(anchor->box_subclasses));
+        "box_subclasses refcount (%lp) %d", anchor->box_subclasses, rb_box_subclasses_ref_count(anchor->box_subclasses));
     st_delete(tbl, &box_id, NULL);
     rb_box_subclasses_ref_dec(anchor->box_subclasses);
     xfree(anchor);
diff --git a/internal/class.h b/internal/class.h
@@ -28,29 +28,29 @@
 #endif
 
 struct rb_box_subclasses {
-    rb_atomic_t refcount;
+    long refcount;
     struct st_table *tbl;
 };
 typedef struct rb_box_subclasses rb_box_subclasses_t;
 
-static inline rb_atomic_t
+static inline long
 rb_box_subclasses_ref_count(rb_box_subclasses_t *box_sub)
 {
-    return ATOMIC_LOAD_RELAXED(box_sub->refcount);
+    return box_sub->refcount;
 }
 
 static inline rb_box_subclasses_t *
 rb_box_subclasses_ref_inc(rb_box_subclasses_t *box_sub)
 {
-    RUBY_ATOMIC_FETCH_ADD(box_sub->refcount, 1);
+    box_sub->refcount++;
     return box_sub;
 }
 
 static inline void
 rb_box_subclasses_ref_dec(rb_box_subclasses_t *box_sub)
 {
-    rb_atomic_t was = RUBY_ATOMIC_FETCH_SUB(box_sub->refcount, 1);
-    if (was == 1) {
+    box_sub->refcount--;
+    if (box_sub->refcount == 0) {
         st_free_table(box_sub->tbl);
         xfree(box_sub);
     }
diff --git a/test/ruby/test_class.rb b/test/ruby/test_class.rb
@@ -887,19 +887,4 @@ def test_method_table_assignment_just_after_class_init
       class C; end
     end;
   end
-
-  def test_subclasses_refcount_in_ractors
-    assert_ractor "#{<<~"begin;"}\n#{<<~'end;'}"
-    begin;
-      rs = []
-      8.times do
-        rs << Ractor.new do
-          5_000.times do
-            Class.new
-          end
-        end
-      end
-      rs.each(&:join)
-    end;
-  end
 end

Original file line number	Diff line number	Diff line change
`@@ -671,7 +671,7 @@ rb_class_classext_free_subclasses(rb_classext_t *ext, VALUE klass, bool replacin`
`671`	`671`	`}`
`672`	`672`	`VM_ASSERT(`
`673`	`673`	`rb_box_subclasses_ref_count(anchor->box_subclasses) > 0,`
`674`		`- "box_subclasses refcount (%p) %d", anchor->box_subclasses, rb_box_subclasses_ref_count(anchor->box_subclasses));`
	`674`	`+ "box_subclasses refcount (%lp) %d", anchor->box_subclasses, rb_box_subclasses_ref_count(anchor->box_subclasses));`
`675`	`675`	`st_delete(tbl, &box_id, NULL);`
`676`	`676`	`rb_box_subclasses_ref_dec(anchor->box_subclasses);`
`677`	`677`	`xfree(anchor);`
Original file line number	Diff line number	Diff line change
`@@ -28,29 +28,29 @@`
`28`	`28`	`#endif`
`29`	`29`
`30`	`30`	`struct rb_box_subclasses {`
`31`		`- rb_atomic_t refcount;`
	`31`	`+ long refcount;`
`32`	`32`	`struct st_table *tbl;`
`33`	`33`	`};`
`34`	`34`	`typedef struct rb_box_subclasses rb_box_subclasses_t;`
`35`	`35`
`36`		`-static inline rb_atomic_t`
	`36`	`+static inline long`
`37`	`37`	`rb_box_subclasses_ref_count(rb_box_subclasses_t *box_sub)`
`38`	`38`	`{`
`39`		`- return ATOMIC_LOAD_RELAXED(box_sub->refcount);`
	`39`	`+ return box_sub->refcount;`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`static inline rb_box_subclasses_t *`
`43`	`43`	`rb_box_subclasses_ref_inc(rb_box_subclasses_t *box_sub)`
`44`	`44`	`{`
`45`		`- RUBY_ATOMIC_FETCH_ADD(box_sub->refcount, 1);`
	`45`	`+ box_sub->refcount++;`
`46`	`46`	`return box_sub;`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`static inline void`
`50`	`50`	`rb_box_subclasses_ref_dec(rb_box_subclasses_t *box_sub)`
`51`	`51`	`{`
`52`		`- rb_atomic_t was = RUBY_ATOMIC_FETCH_SUB(box_sub->refcount, 1);`
`53`		`- if (was == 1) {`
	`52`	`+ box_sub->refcount--;`
	`53`	`+ if (box_sub->refcount == 0) {`
`54`	`54`	`st_free_table(box_sub->tbl);`
`55`	`55`	`xfree(box_sub);`
`56`	`56`	`}`