From d5c8fd2043f4425c3fe2a87558dfbd80ebff9911 Mon Sep 17 00:00:00 2001 From: nagachika Date: Sat, 14 Jun 2025 11:39:08 +0900 Subject: [PATCH] merge revision(s) f6cbf499bc98b851034fffb49fcbb59d495f6f7b: [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] --- bootstraptest/test_ractor.rb | 12 ++++++++++++ common.mk | 2 ++ proc.c | 34 +++++++++++++++++++--------------- version.h | 2 +- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index ea7c2c197d..c03fbb07d7 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1773,3 +1773,15 @@ assert_equal 'ok', %q{ } end # if !ENV['GITHUB_WORKFLOW'] + +# Using Symbol#to_proc inside ractors +# [Bug #21354] +assert_equal 'ok', %q{ + :inspect.to_proc + Ractor.new do + # It should not use this cached proc, it should create a new one. If it used + # the cached proc, we would get a ractor_confirm_belonging error here. + :inspect.to_proc + end.take + 'ok' +} diff --git a/common.mk b/common.mk index 714454de60..4cc436dc41 100644 --- a/common.mk +++ b/common.mk @@ -12804,6 +12804,8 @@ proc.$(OBJEXT): {$(VPATH)}prism/ast.h proc.$(OBJEXT): {$(VPATH)}prism/version.h proc.$(OBJEXT): {$(VPATH)}prism_compile.h proc.$(OBJEXT): {$(VPATH)}proc.c +proc.$(OBJEXT): {$(VPATH)}ractor.h +proc.$(OBJEXT): {$(VPATH)}ractor_core.h proc.$(OBJEXT): {$(VPATH)}ruby_assert.h proc.$(OBJEXT): {$(VPATH)}ruby_atomic.h proc.$(OBJEXT): {$(VPATH)}rubyparser.h diff --git a/proc.c b/proc.c index a3fdb1783c..f09f5ec4d8 100644 --- a/proc.c +++ b/proc.c @@ -22,6 +22,7 @@ #include "method.h" #include "iseq.h" #include "vm_core.h" +#include "ractor_core.h" #include "yjit.h" const rb_cref_t *rb_vm_cref_in_context(VALUE self, VALUE cbase); @@ -1508,23 +1509,26 @@ rb_sym_to_proc(VALUE sym) long index; ID id; - if (!sym_proc_cache) { - sym_proc_cache = rb_ary_hidden_new(SYM_PROC_CACHE_SIZE * 2); - rb_gc_register_mark_object(sym_proc_cache); - rb_ary_store(sym_proc_cache, SYM_PROC_CACHE_SIZE*2 - 1, Qnil); - } - id = SYM2ID(sym); - index = (id % SYM_PROC_CACHE_SIZE) << 1; - if (RARRAY_AREF(sym_proc_cache, index) == sym) { - return RARRAY_AREF(sym_proc_cache, index + 1); - } - else { - proc = sym_proc_new(rb_cProc, ID2SYM(id)); - RARRAY_ASET(sym_proc_cache, index, sym); - RARRAY_ASET(sym_proc_cache, index + 1, proc); - return proc; + if (rb_ractor_main_p()) { + index = (id % SYM_PROC_CACHE_SIZE) << 1; + if (!sym_proc_cache) { + sym_proc_cache = rb_ary_hidden_new(SYM_PROC_CACHE_SIZE * 2); + rb_gc_register_mark_object(sym_proc_cache); + rb_ary_store(sym_proc_cache, SYM_PROC_CACHE_SIZE*2 - 1, Qnil); + } + if (RARRAY_AREF(sym_proc_cache, index) == sym) { + return RARRAY_AREF(sym_proc_cache, index + 1); + } + else { + proc = sym_proc_new(rb_cProc, ID2SYM(id)); + RARRAY_ASET(sym_proc_cache, index, sym); + RARRAY_ASET(sym_proc_cache, index + 1, proc); + return proc; + } + } else { + return sym_proc_new(rb_cProc, ID2SYM(id)); } } diff --git a/version.h b/version.h index d5294f759c..27d6146ac7 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 8 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 156 +#define RUBY_PATCHLEVEL 157 #include "ruby/version.h" #include "ruby/internal/abi.h"