Skip to content

Commit cbcbbb2

Browse files
osyoyujhawthorn
authored andcommitted
Let Ractor::IsolationError report correct constant path
Before this patch, Ractor::IsolationError reported an incorrect constant path when constant was found through `rb_const_get_0()`. In this code, Ractor::IsolationError reported illegal access against `M::TOPLEVEL`, where it should be `Object::TOPLEVEL`. ```ruby TOPLEVEL = [1] module M def self.f TOPLEVEL end end Ractor.new { M.f }.value ``` This was because `rb_const_get_0()` built the "path" part referring to the module/class passed to it in the first place. When a constant was found through recursive search upwards, the module/class which the constant was found should be reported. This patch fixes this issue by modifying rb_const_search() to take a VALUE pointer to be filled with the module/class where the constant was found. [Bug #21782]
1 parent 2d01406 commit cbcbbb2

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

bootstraptest/test_ractor.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,20 @@ def str; STR; end
10891089
end
10901090
RUBY
10911091

1092+
# The correct constant path shall be reported
1093+
assert_equal "can not access non-shareable objects in constant Object::STR by non-main Ractor.", <<~'RUBY', frozen_string_literal: false
1094+
STR = "hello"
1095+
module M
1096+
def self.str; STR; end
1097+
end
1098+
1099+
begin
1100+
Ractor.new{ M.str }.join
1101+
rescue Ractor::RemoteError => e
1102+
e.cause.message
1103+
end
1104+
RUBY
1105+
10921106
# Setting non-shareable objects into constants by other Ractors is not allowed
10931107
assert_equal 'can not set constants with non-shareable objects by non-main Ractors', <<~'RUBY', frozen_string_literal: false
10941108
class C

variable.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ static VALUE autoload_mutex;
6363

6464
static void check_before_mod_set(VALUE, ID, VALUE, const char *);
6565
static void setup_const_entry(rb_const_entry_t *, VALUE, VALUE, rb_const_flag_t);
66-
static VALUE rb_const_search(VALUE klass, ID id, int exclude, int recurse, int visibility);
66+
static VALUE rb_const_search(VALUE klass, ID id, int exclude, int recurse, int visibility, VALUE *found_in);
6767
static st_table *generic_fields_tbl_;
6868

6969
typedef int rb_ivar_foreach_callback_func(ID key, VALUE val, st_data_t arg);
@@ -473,7 +473,7 @@ rb_path_to_class(VALUE pathname)
473473
if (!id) {
474474
goto undefined_class;
475475
}
476-
c = rb_const_search(c, id, TRUE, FALSE, FALSE);
476+
c = rb_const_search(c, id, TRUE, FALSE, FALSE, NULL);
477477
if (UNDEF_P(c)) goto undefined_class;
478478
if (!rb_namespace_p(c)) {
479479
rb_raise(rb_eTypeError, "%"PRIsVALUE" does not refer to class/module",
@@ -3352,11 +3352,12 @@ rb_const_warn_if_deprecated(const rb_const_entry_t *ce, VALUE klass, ID id)
33523352
static VALUE
33533353
rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
33543354
{
3355-
VALUE c = rb_const_search(klass, id, exclude, recurse, visibility);
3355+
VALUE found_in;
3356+
VALUE c = rb_const_search(klass, id, exclude, recurse, visibility, &found_in);
33563357
if (!UNDEF_P(c)) {
33573358
if (UNLIKELY(!rb_ractor_main_p())) {
33583359
if (!rb_ractor_shareable_p(c)) {
3359-
rb_raise(rb_eRactorIsolationError, "can not access non-shareable objects in constant %"PRIsVALUE"::%s by non-main Ractor.", rb_class_path(klass), rb_id2name(id));
3360+
rb_raise(rb_eRactorIsolationError, "can not access non-shareable objects in constant %"PRIsVALUE"::%s by non-main Ractor.", rb_class_path(found_in), rb_id2name(id));
33603361
}
33613362
}
33623363
return c;
@@ -3365,7 +3366,7 @@ rb_const_get_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
33653366
}
33663367

33673368
static VALUE
3368-
rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibility)
3369+
rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibility, VALUE *found_in)
33693370
{
33703371
VALUE value, current;
33713372
bool first_iteration = true;
@@ -3402,13 +3403,17 @@ rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibilit
34023403
if (am == tmp) break;
34033404
am = tmp;
34043405
ac = autoloading_const_entry(tmp, id);
3405-
if (ac) return ac->value;
3406+
if (ac) {
3407+
if (found_in) { *found_in = tmp; }
3408+
return ac->value;
3409+
}
34063410
rb_autoload_load(tmp, id);
34073411
continue;
34083412
}
34093413
if (exclude && tmp == rb_cObject) {
34103414
goto not_found;
34113415
}
3416+
if (found_in) { *found_in = tmp; }
34123417
return value;
34133418
}
34143419
if (!recurse) break;
@@ -3420,17 +3425,17 @@ rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibilit
34203425
}
34213426

34223427
static VALUE
3423-
rb_const_search(VALUE klass, ID id, int exclude, int recurse, int visibility)
3428+
rb_const_search(VALUE klass, ID id, int exclude, int recurse, int visibility, VALUE *found_in)
34243429
{
34253430
VALUE value;
34263431

34273432
if (klass == rb_cObject) exclude = FALSE;
3428-
value = rb_const_search_from(klass, id, exclude, recurse, visibility);
3433+
value = rb_const_search_from(klass, id, exclude, recurse, visibility, found_in);
34293434
if (!UNDEF_P(value)) return value;
34303435
if (exclude) return value;
34313436
if (BUILTIN_TYPE(klass) != T_MODULE) return value;
34323437
/* search global const too, if klass is a module */
3433-
return rb_const_search_from(rb_cObject, id, FALSE, recurse, visibility);
3438+
return rb_const_search_from(rb_cObject, id, FALSE, recurse, visibility, found_in);
34343439
}
34353440

34363441
VALUE

0 commit comments

Comments
 (0)