Skip to content

Commit 2998c8d

Browse files
authored
ns_subclasses refcount accesses need to be atomic (ruby#15083)
We were seeing errors like: ``` * thread #8, stop reason = EXC_BAD_ACCESS (code=1, address=0x803) * frame #0: 0x00000001001fe944 ruby`rb_st_lookup(tab=0x00000000000007fb, key=1, value=0x00000001305b7490) at st.c:1066:22 frame #1: 0x000000010002d658 ruby`remove_class_from_subclasses [inlined] class_get_subclasses_for_ns(tbl=0x00000000000007fb, ns_id=1) at class.c:604:9 frame #2: 0x000000010002d650 ruby`remove_class_from_subclasses(tbl=0x00000000000007fb, ns_id=1, klass=4754039232) at class.c:620:34 frame #3: 0x000000010002c8a8 ruby`rb_class_classext_free_subclasses(ext=0x000000011b5ce1d8, klass=4754039232, replacing=<unavailable>) at class.c:700:9 frame #4: 0x000000010002c760 ruby`rb_class_classext_free(klass=4754039232, ext=0x000000011b5ce1d8, is_prime=true) at class.c:105:5 frame #5: 0x00000001000e770c ruby`classext_free(ext=<unavailable>, is_prime=<unavailable>, namespace=<unavailable>, arg=<unavailable>) at gc.c:1231:5 [artificial] frame #6: 0x000000010002d178 ruby`rb_class_classext_foreach(klass=<unavailable>, func=(ruby`classext_free at gc.c:1228), arg=0x00000001305b75c0) at class.c:518:5 frame #7: 0x00000001000e745c ruby`rb_gc_obj_free(objspace=0x000000012500c400, obj=4754039232) at gc.c:1282:9 frame #8: 0x00000001000e70d4 ruby`gc_sweep_plane(objspace=0x000000012500c400, heap=<unavailable>, p=4754039232, bitset=4095, ctx=0x00000001305b76e8) at default.c:3482:21 frame #9: 0x00000001000e6e9c ruby`gc_sweep_page(objspace=0x000000012500c400, heap=0x000000012500c540, ctx=0x00000001305b76e8) at default.c:3567:13 frame #10: 0x00000001000e51d0 ruby`gc_sweep_step(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:3848:9 frame #11: 0x00000001000e1880 ruby`gc_continue [inlined] gc_sweep_continue(objspace=0x000000012500c400, sweep_heap=0x000000012500c540) at default.c:3931:13 frame #12: 0x00000001000e1754 ruby`gc_continue(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2037:9 frame #13: 0x00000001000e10bc ruby`newobj_cache_miss [inlined] heap_prepare(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2056:5 frame #14: 0x00000001000e1074 ruby`newobj_cache_miss [inlined] heap_next_free_page(objspace=0x000000012500c400, heap=0x000000012500c540) at default.c:2280:9 frame #15: 0x00000001000e106c ruby`newobj_cache_miss(objspace=0x000000012500c400, cache=0x0000600001b00300, heap_idx=2, vm_locked=false) at default.c:2387:38 frame #16: 0x00000001000e0d28 ruby`newobj_alloc(objspace=<unavailable>, cache=<unavailable>, heap_idx=<unavailable>, vm_locked=<unavailable>) at default.c:2411:15 [artificial] frame #17: 0x00000001000d7214 ruby`newobj_of [inlined] rb_gc_impl_new_obj(objspace_ptr=<unavailable>, cache_ptr=<unavailable>, klass=<unavailable>, flags=<unavailable>, wb_protected=<unavailable>, alloc_size=<unavailable>) at default.c:2490:15 frame #18: 0x00000001000d719c ruby`newobj_of(cr=<unavailable>, klass=4313971728, flags=258, wb_protected=<unavailable>, size=<unavailable>) at gc.c:995:17 frame #19: 0x00000001000d73ec ruby`rb_wb_protected_newobj_of(ec=<unavailable>, klass=<unavailable>, flags=<unavailable>, size=<unavailable>) at gc.c:1044:12 [artificial] frame #20: 0x0000000100032d34 ruby`class_alloc0(type=<unavailable>, klass=4313971728, namespaceable=<unavailable>) at class.c:803:5 ```
1 parent 38d31dc commit 2998c8d

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

class.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,7 @@ rb_class_classext_free_subclasses(rb_classext_t *ext, VALUE klass, bool replacin
690690
}
691691
VM_ASSERT(
692692
rb_ns_subclasses_ref_count(anchor->ns_subclasses) > 0,
693-
"ns_subclasses refcount (%p) %ld", anchor->ns_subclasses, rb_ns_subclasses_ref_count(anchor->ns_subclasses));
693+
"ns_subclasses refcount (%p) %d", anchor->ns_subclasses, rb_ns_subclasses_ref_count(anchor->ns_subclasses));
694694
st_delete(tbl, &ns_id, NULL);
695695
rb_ns_subclasses_ref_dec(anchor->ns_subclasses);
696696
xfree(anchor);

internal/class.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,29 +28,29 @@
2828
#endif
2929

3030
struct rb_ns_subclasses {
31-
long refcount;
31+
rb_atomic_t refcount;
3232
struct st_table *tbl;
3333
};
3434
typedef struct rb_ns_subclasses rb_ns_subclasses_t;
3535

36-
static inline long
36+
static inline rb_atomic_t
3737
rb_ns_subclasses_ref_count(rb_ns_subclasses_t *ns_sub)
3838
{
39-
return ns_sub->refcount;
39+
return ATOMIC_LOAD_RELAXED(ns_sub->refcount);
4040
}
4141

4242
static inline rb_ns_subclasses_t *
4343
rb_ns_subclasses_ref_inc(rb_ns_subclasses_t *ns_sub)
4444
{
45-
ns_sub->refcount++;
45+
RUBY_ATOMIC_FETCH_ADD(ns_sub->refcount, 1);
4646
return ns_sub;
4747
}
4848

4949
static inline void
5050
rb_ns_subclasses_ref_dec(rb_ns_subclasses_t *ns_sub)
5151
{
52-
ns_sub->refcount--;
53-
if (ns_sub->refcount == 0) {
52+
rb_atomic_t was = RUBY_ATOMIC_FETCH_SUB(ns_sub->refcount, 1);
53+
if (was == 1) {
5454
st_free_table(ns_sub->tbl);
5555
xfree(ns_sub);
5656
}

test/ruby/test_class.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,4 +887,19 @@ def test_method_table_assignment_just_after_class_init
887887
class C; end
888888
end;
889889
end
890+
891+
def test_subclasses_refcount_in_ractors
892+
assert_ractor "#{<<~"begin;"}\n#{<<~'end;'}"
893+
begin;
894+
rs = []
895+
8.times do
896+
rs << Ractor.new do
897+
5_000.times do
898+
Class.new
899+
end
900+
end
901+
end
902+
rs.each(&:join)
903+
end;
904+
end
890905
end

0 commit comments

Comments
 (0)