Skip to content

Commit 482f4ca

Browse files
committed
Autoload encodings on the main ractor
None of the datastructures involved in the require process are safe to call on a secondary ractor, however when autoloading encodings, we do so from the current ractor. So all sorts of corruption can happen when using an autoloaded encoding for the first time from a secondary ractor.
1 parent 002d746 commit 482f4ca

File tree

5 files changed

+81
-19
lines changed

5 files changed

+81
-19
lines changed

encoding.c

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -763,36 +763,47 @@ load_encoding(const char *name)
763763
}
764764

765765
static int
766-
enc_autoload_body(struct enc_table *enc_table, rb_encoding *enc)
766+
enc_autoload_body(rb_encoding *enc)
767767
{
768-
rb_encoding *base = enc_table->list[ENC_TO_ENCINDEX(enc)].base;
768+
rb_encoding *base;
769+
int i = 0;
770+
771+
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
772+
base = enc_table->list[ENC_TO_ENCINDEX(enc)].base;
773+
if (base) {
774+
do {
775+
if (i >= enc_table->count) {
776+
i = -1;
777+
break;
778+
}
779+
} while (enc_table->list[i].enc != base && (++i, 1));
780+
}
781+
}
782+
783+
if (i == -1) return -1;
769784

770785
if (base) {
771-
int i = 0;
772-
do {
773-
if (i >= enc_table->count) return -1;
774-
} while (enc_table->list[i].enc != base && (++i, 1));
775786
if (rb_enc_autoload_p(base)) {
776787
if (rb_enc_autoload(base) < 0) return -1;
777788
}
778789
i = enc->ruby_encoding_index;
779-
enc_register_at(enc_table, i & ENC_INDEX_MASK, rb_enc_name(enc), base);
790+
791+
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
792+
enc_register_at(enc_table, i & ENC_INDEX_MASK, rb_enc_name(enc), base);
793+
}
794+
780795
((rb_raw_encoding *)enc)->ruby_encoding_index = i;
781796
i &= ENC_INDEX_MASK;
782797
return i;
783798
}
784-
else {
785-
return -2;
786-
}
799+
800+
return -2;
787801
}
788802

789803
int
790804
rb_enc_autoload(rb_encoding *enc)
791805
{
792-
int i;
793-
GLOBAL_ENC_TABLE_LOCKING(enc_table) {
794-
i = enc_autoload_body(enc_table, enc);
795-
}
806+
int i = enc_autoload_body(enc);
796807
if (i == -2) {
797808
i = load_encoding(rb_enc_name(enc));
798809
}

load.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,8 @@ features_index_add_single(vm_ns_t *vm_ns, const char* str, size_t len, VALUE off
372372
static void
373373
features_index_add(vm_ns_t *vm_ns, VALUE feature, VALUE offset)
374374
{
375+
RUBY_ASSERT(rb_ractor_main_p());
376+
375377
const char *feature_str, *feature_end, *ext, *p;
376378
bool rb = false;
377379

@@ -1523,6 +1525,10 @@ require_internal(rb_execution_context_t *ec, VALUE fname, int exception, bool wa
15231525
int
15241526
rb_require_internal_silent(VALUE fname)
15251527
{
1528+
if (!rb_ractor_main_p()) {
1529+
return NUM2INT(rb_ractor_require(fname, true));
1530+
}
1531+
15261532
rb_execution_context_t *ec = GET_EC();
15271533
return require_internal(ec, fname, 1, false);
15281534
}
@@ -1559,7 +1565,7 @@ rb_require_string_internal(VALUE fname, bool resurrect)
15591565
// main ractor check
15601566
if (!rb_ractor_main_p()) {
15611567
if (resurrect) fname = rb_str_resurrect(fname);
1562-
return rb_ractor_require(fname);
1568+
return rb_ractor_require(fname, false);
15631569
}
15641570
else {
15651571
int result = require_internal(ec, fname, 1, RTEST(ruby_verbose));

ractor.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,6 +2263,8 @@ struct cross_ractor_require {
22632263
// autoload
22642264
VALUE module;
22652265
ID name;
2266+
2267+
bool silent;
22662268
};
22672269

22682270
static void
@@ -2294,7 +2296,14 @@ require_body(VALUE data)
22942296

22952297
ID require;
22962298
CONST_ID(require, "require");
2297-
crr->result = rb_funcallv(Qnil, require, 1, &crr->feature);
2299+
2300+
if (crr->silent) {
2301+
int rb_require_internal_silent(VALUE fname);
2302+
crr->result = INT2NUM(rb_require_internal_silent(crr->feature));
2303+
}
2304+
else {
2305+
crr->result = rb_funcallv(Qnil, require, 1, &crr->feature);
2306+
}
22982307

22992308
return Qnil;
23002309
}
@@ -2338,10 +2347,21 @@ ractor_require_protect(VALUE crr_obj, VALUE (*func)(VALUE))
23382347
struct cross_ractor_require *crr;
23392348
TypedData_Get_Struct(crr_obj, struct cross_ractor_require, &cross_ractor_require_data_type, crr);
23402349

2350+
VALUE debug, errinfo;
2351+
if (crr->silent) {
2352+
debug = ruby_debug;
2353+
errinfo = rb_errinfo();
2354+
}
2355+
23412356
// catch any error
23422357
rb_rescue2(func, (VALUE)crr,
23432358
require_rescue, (VALUE)crr, rb_eException, 0);
23442359

2360+
if (crr->silent) {
2361+
ruby_debug = debug;
2362+
rb_set_errinfo(errinfo);
2363+
}
2364+
23452365
rb_rescue2(require_result_copy_body, (VALUE)crr,
23462366
require_result_copy_resuce, (VALUE)crr, rb_eException, 0);
23472367

@@ -2357,8 +2377,11 @@ ractor_require_func(void *crr_obj)
23572377
}
23582378

23592379
VALUE
2360-
rb_ractor_require(VALUE feature)
2380+
rb_ractor_require(VALUE feature, bool silent)
23612381
{
2382+
// We're about to block on the main ractor, so if we're holding the global lock we'll deadlock.
2383+
ASSERT_vm_unlocking();
2384+
23622385
struct cross_ractor_require *crr;
23632386
VALUE crr_obj = TypedData_Make_Struct(0, struct cross_ractor_require, &cross_ractor_require_data_type, crr);
23642387
FL_SET_RAW(crr_obj, RUBY_FL_SHAREABLE);
@@ -2368,6 +2391,7 @@ rb_ractor_require(VALUE feature)
23682391
crr->port = ractor_port_new(GET_RACTOR());
23692392
crr->result = Qundef;
23702393
crr->exception = Qundef;
2394+
crr->silent = silent;
23712395

23722396
rb_execution_context_t *ec = GET_EC();
23732397
rb_ractor_t *main_r = GET_VM()->ractor.main_ractor;
@@ -2395,7 +2419,7 @@ rb_ractor_require(VALUE feature)
23952419
static VALUE
23962420
ractor_require(rb_execution_context_t *ec, VALUE self, VALUE feature)
23972421
{
2398-
return rb_ractor_require(feature);
2422+
return rb_ractor_require(feature, false);
23992423
}
24002424

24012425
static VALUE

ractor_core.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ void rb_ractor_terminate_all(void);
134134
bool rb_ractor_main_p_(void);
135135
void rb_ractor_atfork(rb_vm_t *vm, rb_thread_t *th);
136136
void rb_ractor_terminate_atfork(rb_vm_t *vm, rb_ractor_t *th);
137-
VALUE rb_ractor_require(VALUE feature);
137+
VALUE rb_ractor_require(VALUE feature, bool silent);
138138
VALUE rb_ractor_autoload_load(VALUE space, ID id);
139139

140140
VALUE rb_ractor_ensure_shareable(VALUE obj, VALUE name);

test/ruby/test_encoding.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,4 +136,25 @@ def test_ractor_load_encoding
136136
assert "[Bug #19562]"
137137
end;
138138
end
139+
140+
def test_ractor_lazy_load_encoding_concurrently
141+
assert_ractor("#{<<~"begin;"}\n#{<<~'end;'}")
142+
begin;
143+
rs = []
144+
autoload_encodings = Encoding.list.select { |e| e.inspect.include?("(autoload)") }.freeze
145+
7.times do
146+
rs << Ractor.new(autoload_encodings) do |encodings|
147+
str = "abc".dup
148+
encodings.each do |enc|
149+
str.force_encoding(enc)
150+
end
151+
end
152+
end
153+
while rs.any?
154+
r, _obj = Ractor.select(*rs)
155+
rs.delete(r)
156+
end
157+
assert rs.empty?
158+
end;
159+
end
139160
end

0 commit comments

Comments
 (0)