Skip to content

Commit 82e05dc

Browse files
committed
merge revision(s) f6cbf49: [Backport #21354]
Fix Symbol#to_proc (rb_sym_to_proc) to be ractor safe In non-main ractors, don't use `sym_proc_cache`. It is not thread-safe to add to this array without a lock and also it leaks procs from one ractor to another. Instead, we create a new proc each time. If this results in poor performance we can come up with a solution later. Fixes [Bug #21354]
1 parent 585469a commit 82e05dc

File tree

4 files changed

+34
-16
lines changed

4 files changed

+34
-16
lines changed

bootstraptest/test_ractor.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,3 +1939,15 @@ def require feature
19391939
end
19401940
r.take
19411941
}
1942+
1943+
# Using Symbol#to_proc inside ractors
1944+
# [Bug #21354]
1945+
assert_equal 'ok', %q{
1946+
:inspect.to_proc
1947+
Ractor.new do
1948+
# It should not use this cached proc, it should create a new one. If it used
1949+
# the cached proc, we would get a ractor_confirm_belonging error here.
1950+
:inspect.to_proc
1951+
end.take
1952+
'ok'
1953+
}

common.mk

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13298,6 +13298,8 @@ proc.$(OBJEXT): {$(VPATH)}prism/diagnostic.h
1329813298
proc.$(OBJEXT): {$(VPATH)}prism/version.h
1329913299
proc.$(OBJEXT): {$(VPATH)}prism_compile.h
1330013300
proc.$(OBJEXT): {$(VPATH)}proc.c
13301+
proc.$(OBJEXT): {$(VPATH)}ractor.h
13302+
proc.$(OBJEXT): {$(VPATH)}ractor_core.h
1330113303
proc.$(OBJEXT): {$(VPATH)}ruby_assert.h
1330213304
proc.$(OBJEXT): {$(VPATH)}ruby_atomic.h
1330313305
proc.$(OBJEXT): {$(VPATH)}rubyparser.h

proc.c

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "method.h"
2323
#include "iseq.h"
2424
#include "vm_core.h"
25+
#include "ractor_core.h"
2526
#include "yjit.h"
2627

2728
const rb_cref_t *rb_vm_cref_in_context(VALUE self, VALUE cbase);
@@ -1521,23 +1522,26 @@ rb_sym_to_proc(VALUE sym)
15211522
long index;
15221523
ID id;
15231524

1524-
if (!sym_proc_cache) {
1525-
sym_proc_cache = rb_ary_hidden_new(SYM_PROC_CACHE_SIZE * 2);
1526-
rb_vm_register_global_object(sym_proc_cache);
1527-
rb_ary_store(sym_proc_cache, SYM_PROC_CACHE_SIZE*2 - 1, Qnil);
1528-
}
1529-
15301525
id = SYM2ID(sym);
1531-
index = (id % SYM_PROC_CACHE_SIZE) << 1;
15321526

1533-
if (RARRAY_AREF(sym_proc_cache, index) == sym) {
1534-
return RARRAY_AREF(sym_proc_cache, index + 1);
1535-
}
1536-
else {
1537-
proc = sym_proc_new(rb_cProc, ID2SYM(id));
1538-
RARRAY_ASET(sym_proc_cache, index, sym);
1539-
RARRAY_ASET(sym_proc_cache, index + 1, proc);
1540-
return proc;
1527+
if (rb_ractor_main_p()) {
1528+
index = (id % SYM_PROC_CACHE_SIZE) << 1;
1529+
if (!sym_proc_cache) {
1530+
sym_proc_cache = rb_ary_hidden_new(SYM_PROC_CACHE_SIZE * 2);
1531+
rb_vm_register_global_object(sym_proc_cache);
1532+
rb_ary_store(sym_proc_cache, SYM_PROC_CACHE_SIZE*2 - 1, Qnil);
1533+
}
1534+
if (RARRAY_AREF(sym_proc_cache, index) == sym) {
1535+
return RARRAY_AREF(sym_proc_cache, index + 1);
1536+
}
1537+
else {
1538+
proc = sym_proc_new(rb_cProc, ID2SYM(id));
1539+
RARRAY_ASET(sym_proc_cache, index, sym);
1540+
RARRAY_ASET(sym_proc_cache, index + 1, proc);
1541+
return proc;
1542+
}
1543+
} else {
1544+
return sym_proc_new(rb_cProc, ID2SYM(id));
15411545
}
15421546
}
15431547

version.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
1212
#define RUBY_VERSION_TEENY 4
1313
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
14-
#define RUBY_PATCHLEVEL 42
14+
#define RUBY_PATCHLEVEL 43
1515

1616
#include "ruby/version.h"
1717
#include "ruby/internal/abi.h"

0 commit comments

Comments
 (0)