1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use atomic for method reference count [Bug #20934]

Browse Source 
This changes reference_count on rb_method_definition_struct into an
atomic.

Ractors can create additional references as part of `bind_call` or
(presumably) similar. Because this can be done inside Ractors, we should
use a lock or atomics so that we don't race and avoid incrementing.

Co-authored-by: wanabe <s.wanabe@gmail.com>
This commit is contained in:
John Hawthorn 2025-03-18 15:02:44 -07:00
parent 62cc3464d9
commit bfe6068417
Notes: git 2025-03-20 20:09:57 +00:00 
Merged: https://github.com/ruby/ruby/pull/12951
3 changed files with 29 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
bootstraptest/test_ractor.rb
View File

@ -1937,3 +1937,16 @@ assert_equal 'LoadError', %q{
end end
r.take r.take
} }
# bind_call in Ractor [Bug #20934]
assert_equal 'ok', %q{
2.times.map do
Ractor.new do
1000.times do
Object.instance_method(:itself).bind_call(self)
end
end
end.each(&:take)
GC.start
:ok.itself
}

4
method.h
View File

@ -15,6 +15,7 @@
#include "internal/imemo.h" #include "internal/imemo.h"
#include "internal/compilers.h" #include "internal/compilers.h"
#include "internal/static_assert.h" #include "internal/static_assert.h"
#include "ruby/atomic.h"
#ifndef END_OF_ENUMERATION #ifndef END_OF_ENUMERATION
# if defined(__GNUC__) &&! defined(__STRICT_ANSI__) # if defined(__GNUC__) &&! defined(__STRICT_ANSI__)
@ -181,7 +182,8 @@ struct rb_method_definition_struct {
unsigned int iseq_overload: 1; unsigned int iseq_overload: 1;
unsigned int no_redef_warning: 1; unsigned int no_redef_warning: 1;
unsigned int aliased : 1; unsigned int aliased : 1;
int reference_count : 28;
rb_atomic_t reference_count;
union { union {
rb_method_iseq_t iseq; rb_method_iseq_t iseq;

23
vm_method.c
View File

@ -511,14 +511,13 @@ static void
rb_method_definition_release(rb_method_definition_t *def) rb_method_definition_release(rb_method_definition_t *def)
{ {
if (def != NULL) { if (def != NULL) {
const int reference_count = def->reference_count; const unsigned int reference_count_was = RUBY_ATOMIC_FETCH_SUB(def->reference_count, 1);
def->reference_count--;
VM_ASSERT(reference_count >= 0); RUBY_ASSERT_ALWAYS(reference_count_was != 0);
if (def->reference_count == 0) { if (reference_count_was == 1) {
if (METHOD_DEBUG) fprintf(stderr, "-%p-%s:%d (remove)\n", (void *)def, if (METHOD_DEBUG) fprintf(stderr, "-%p-%s:1->0 (remove)\n", (void *)def,
rb_id2name(def->original_id), def->reference_count); rb_id2name(def->original_id));
if (def->type == VM_METHOD_TYPE_BMETHOD && def->body.bmethod.hooks) { if (def->type == VM_METHOD_TYPE_BMETHOD && def->body.bmethod.hooks) {
xfree(def->body.bmethod.hooks); xfree(def->body.bmethod.hooks);
} }
@ -526,7 +525,7 @@ rb_method_definition_release(rb_method_definition_t *def)
} }
else { else {
if (METHOD_DEBUG) fprintf(stderr, "-%p-%s:%d->%d (dec)\n", (void *)def, rb_id2name(def->original_id), if (METHOD_DEBUG) fprintf(stderr, "-%p-%s:%d->%d (dec)\n", (void *)def, rb_id2name(def->original_id),
reference_count, def->reference_count); reference_count_was, reference_count_was - 1);
} }
} }
} }
@ -614,9 +613,13 @@ setup_method_cfunc_struct(rb_method_cfunc_t *cfunc, VALUE (*func)(ANYARGS), int
static rb_method_definition_t * static rb_method_definition_t *
method_definition_addref(rb_method_definition_t *def, bool complemented) method_definition_addref(rb_method_definition_t *def, bool complemented)
{ {
if (!complemented && def->reference_count > 0) def->aliased = true; unsigned int reference_count_was = RUBY_ATOMIC_FETCH_ADD(def->reference_count, 1);
def->reference_count++; if (!complemented && reference_count_was > 0) {
if (METHOD_DEBUG) fprintf(stderr, "+%p-%s:%d\n", (void *)def, rb_id2name(def->original_id), def->reference_count); /* TODO: A Ractor can reach this via UnboundMethod#bind */
def->aliased = true;
}
if (METHOD_DEBUG) fprintf(stderr, "+%p-%s:%d->%d\n", (void *)def, rb_id2name(def->original_id), reference_count_was, reference_count_was+1);
return def; return def;
} }