Skip to content

Commit d964b35

Browse files
luke-gruberpeterzhu2118
luke-gruber
authored and
peterzhu2118
committed
Ractors: fixes for parallel const_set/remove_const and const_get
1 parent 1bb15bd commit d964b35

3 files changed

Lines changed: 83 additions & 59 deletions

File tree

‎constant.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ void rb_free_const_table(struct rb_id_table *tbl);
4444
VALUE rb_const_source_location(VALUE, ID);
4545

4646
int rb_autoloading_value(VALUE mod, ID id, VALUE *value, rb_const_flag_t *flag);
47-
rb_const_entry_t *rb_const_lookup(VALUE klass, ID id);
47+
rb_const_entry_t *rb_const_lookup(VALUE klass, ID id, rb_const_entry_t *entry_out);
4848
VALUE rb_public_const_get_at(VALUE klass, ID id);
4949
VALUE rb_public_const_get_from(VALUE klass, ID id);
5050
int rb_public_const_defined_from(VALUE klass, ID id);

‎variable.c‎

Lines changed: 78 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2995,9 +2995,10 @@ static VALUE
29952995
autoload_synchronized(VALUE _arguments)
29962996
{
29972997
struct autoload_arguments *arguments = (struct autoload_arguments *)_arguments;
2998+
rb_const_entry_t constant_entry = {0};
29982999

2999-
rb_const_entry_t *constant_entry = rb_const_lookup(arguments->module, arguments->name);
3000-
if (constant_entry && !UNDEF_P(constant_entry->value)) {
3000+
rb_const_entry_t *ce = rb_const_lookup(arguments->module, arguments->name, &constant_entry);
3001+
if (ce && !UNDEF_P(constant_entry.value)) {
30013002
return Qfalse;
30023003
}
30033004

@@ -3198,13 +3199,15 @@ autoloading_const_entry(VALUE mod, ID id)
31983199
return 0;
31993200
}
32003201

3202+
32013203
static int
32023204
autoload_defined_p(VALUE mod, ID id)
32033205
{
3204-
rb_const_entry_t *ce = rb_const_lookup(mod, id);
3206+
rb_const_entry_t ce_out = {0};
3207+
rb_const_entry_t *ce = rb_const_lookup(mod, id, &ce_out);
32053208

32063209
// If there is no constant or the constant is not undefined (special marker for autoloading):
3207-
if (!ce || !UNDEF_P(ce->value)) {
3210+
if (!ce || !UNDEF_P(ce_out.value)) {
32083211
// We are not autoloading:
32093212
return 0;
32103213
}
@@ -3393,11 +3396,12 @@ autoload_try_load(VALUE _arguments)
33933396
struct autoload_load_arguments *arguments = (struct autoload_load_arguments*)_arguments;
33943397

33953398
VALUE result = autoload_feature_require(_arguments);
3399+
rb_const_entry_t ce_out = {0};
33963400

33973401
// After we loaded the feature, if the constant is not defined, we remove it completely:
3398-
rb_const_entry_t *ce = rb_const_lookup(arguments->module, arguments->name);
3402+
rb_const_entry_t *ce = rb_const_lookup(arguments->module, arguments->name, &ce_out);
33993403

3400-
if (!ce || UNDEF_P(ce->value)) {
3404+
if (!ce || UNDEF_P(ce_out.value)) {
34013405
result = Qfalse;
34023406

34033407
rb_const_remove(arguments->module, arguments->name);
@@ -3429,10 +3433,11 @@ autoload_try_load(VALUE _arguments)
34293433
VALUE
34303434
rb_autoload_load(VALUE module, ID name)
34313435
{
3432-
rb_const_entry_t *ce = rb_const_lookup(module, name);
3436+
rb_const_entry_t ce_out = {0};
3437+
rb_const_entry_t *ce = rb_const_lookup(module, name, &ce_out);
34333438

34343439
// We bail out as early as possible without any synchronisation:
3435-
if (!ce || !UNDEF_P(ce->value)) {
3440+
if (!ce || !UNDEF_P(ce_out.value)) {
34363441
return Qfalse;
34373442
}
34383443

@@ -3450,7 +3455,7 @@ rb_autoload_load(VALUE module, ID name)
34503455
// This confirms whether autoloading is required or not:
34513456
if (autoload_const_value == Qfalse) return autoload_const_value;
34523457

3453-
arguments.flag = ce->flag & (CONST_DEPRECATED | CONST_VISIBILITY_MASK);
3458+
arguments.flag = ce_out.flag & (CONST_DEPRECATED | CONST_VISIBILITY_MASK);
34543459

34553460
// Only one thread will enter here at a time:
34563461
VALUE result = rb_mutex_synchronize(arguments.mutex, autoload_try_load, (VALUE)&arguments);
@@ -3521,6 +3526,7 @@ rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibilit
35213526
{
35223527
VALUE value, current;
35233528
bool first_iteration = true;
3529+
rb_const_entry_t ce_out = {0};
35243530

35253531
for (current = klass;
35263532
RTEST(current);
@@ -3542,13 +3548,13 @@ rb_const_search_from(VALUE klass, ID id, int exclude, int recurse, int visibilit
35423548
if (BUILTIN_TYPE(tmp) == T_ICLASS) tmp = RBASIC(tmp)->klass;
35433549

35443550
// Do the lookup. Loop in case of autoload.
3545-
while ((ce = rb_const_lookup(tmp, id))) {
3546-
if (visibility && RB_CONST_PRIVATE_P(ce)) {
3551+
while ((ce = rb_const_lookup(tmp, id, &ce_out))) {
3552+
if (visibility && RB_CONST_PRIVATE_P(&ce_out)) {
35473553
GET_EC()->private_const_reference = tmp;
35483554
return Qundef;
35493555
}
3550-
rb_const_warn_if_deprecated(ce, tmp, id);
3551-
value = ce->value;
3556+
rb_const_warn_if_deprecated(&ce_out, tmp, id);
3557+
value = ce_out.value;
35523558
if (UNDEF_P(value)) {
35533559
struct autoload_const *ac;
35543560
if (am == tmp) break;
@@ -3628,16 +3634,17 @@ rb_const_location_from(VALUE klass, ID id, int exclude, int recurse, int visibil
36283634
{
36293635
while (RTEST(klass)) {
36303636
rb_const_entry_t *ce;
3637+
rb_const_entry_t ce_out = {0};
36313638

3632-
while ((ce = rb_const_lookup(klass, id))) {
3633-
if (visibility && RB_CONST_PRIVATE_P(ce)) {
3639+
while ((ce = rb_const_lookup(klass, id, &ce_out))) {
3640+
if (visibility && RB_CONST_PRIVATE_P(&ce_out)) {
36343641
return Qnil;
36353642
}
36363643
if (exclude && klass == rb_cObject) {
36373644
goto not_found;
36383645
}
36393646

3640-
if (UNDEF_P(ce->value)) { // autoload
3647+
if (UNDEF_P(ce_out.value)) { // autoload
36413648
VALUE autoload_const_value = autoload_data(klass, id);
36423649
if (RTEST(autoload_const_value)) {
36433650
struct autoload_const *autoload_const;
@@ -3649,8 +3656,8 @@ rb_const_location_from(VALUE klass, ID id, int exclude, int recurse, int visibil
36493656
}
36503657
}
36513658

3652-
if (NIL_P(ce->file)) return rb_ary_new();
3653-
return rb_assoc_new(ce->file, INT2NUM(ce->line));
3659+
if (NIL_P(ce_out.file)) return rb_ary_new();
3660+
return rb_assoc_new(ce_out.file, INT2NUM(ce_out.line));
36543661
}
36553662
if (!recurse) break;
36563663
klass = RCLASS_SUPER(klass);
@@ -3707,18 +3714,19 @@ rb_mod_remove_const(VALUE mod, VALUE name)
37073714
return rb_const_remove(mod, id);
37083715
}
37093716

3710-
static rb_const_entry_t * const_lookup(struct rb_id_table *tbl, ID id);
3717+
static rb_const_entry_t * const_lookup(struct rb_id_table *tbl, ID id, rb_const_entry_t *ce_out);
37113718

37123719
VALUE
37133720
rb_const_remove(VALUE mod, ID id)
37143721
{
37153722
VALUE val;
37163723
rb_const_entry_t *ce;
3724+
rb_const_entry_t ce_out = {0};
37173725

37183726
rb_check_frozen(mod);
37193727

37203728
RB_VM_LOCKING() {
3721-
ce = rb_const_lookup(mod, id);
3729+
ce = rb_const_lookup(mod, id, &ce_out);
37223730
if (!ce || !rb_id_table_delete(RCLASS_WRITABLE_CONST_TBL(mod), id)) {
37233731
if (rb_const_defined_at(mod, id)) {
37243732
rb_name_err_raise("cannot remove %2$s::%1$s", mod, ID2SYM(id));
@@ -3730,14 +3738,14 @@ rb_const_remove(VALUE mod, ID id)
37303738
rb_const_warn_if_deprecated(ce, mod, id);
37313739
rb_clear_constant_cache_for_id(id);
37323740

3733-
val = ce->value;
3741+
val = ce_out.value;
37343742

37353743
if (UNDEF_P(val)) {
37363744
autoload_delete(mod, id);
37373745
val = Qnil;
37383746
}
37393747

3740-
if (ce != const_lookup(RCLASS_PRIME_CONST_TBL(mod), id)) {
3748+
if (ce != const_lookup(RCLASS_PRIME_CONST_TBL(mod), id, NULL)) {
37413749
ruby_xfree(ce);
37423750
}
37433751
// else - skip free'ing the ce because it still exists in the prime classext
@@ -3881,15 +3889,16 @@ rb_const_defined_0(VALUE klass, ID id, int exclude, int recurse, int visibility)
38813889
VALUE tmp;
38823890
int mod_retry = 0;
38833891
rb_const_entry_t *ce;
3892+
rb_const_entry_t ce_out = {0};
38843893

38853894
tmp = klass;
38863895
retry:
38873896
while (tmp) {
3888-
if ((ce = rb_const_lookup(tmp, id))) {
3889-
if (visibility && RB_CONST_PRIVATE_P(ce)) {
3897+
if ((ce = rb_const_lookup(tmp, id, &ce_out))) {
3898+
if (visibility && RB_CONST_PRIVATE_P(&ce_out)) {
38903899
return (int)Qfalse;
38913900
}
3892-
if (UNDEF_P(ce->value) && !check_autoload_required(tmp, id, 0) &&
3901+
if (UNDEF_P(ce_out.value) && !check_autoload_required(tmp, id, 0) &&
38933902
!rb_autoloading_value(tmp, id, NULL, NULL))
38943903
return (int)Qfalse;
38953904

@@ -4017,8 +4026,8 @@ const_set(VALUE klass, ID id, VALUE val)
40174026
RCLASS_WRITE_CONST_TBL(klass, tbl, false);
40184027
rb_clear_constant_cache_for_id(id);
40194028
ce = ZALLOC(rb_const_entry_t);
4020-
rb_id_table_insert(tbl, id, (VALUE)ce);
40214029
setup_const_entry(ce, klass, val, CONST_PUBLIC);
4030+
rb_id_table_insert(tbl, id, (VALUE)ce);
40224031
}
40234032
else {
40244033
struct autoload_const ac = {
@@ -4031,6 +4040,7 @@ const_set(VALUE klass, ID id, VALUE val)
40314040
}
40324041
}
40334042

4043+
// TODO: I think this needs to be locked
40344044
/*
40354045
* Resolve and cache class name immediately to resolve ambiguity
40364046
* and avoid order-dependency on const_tbl
@@ -4095,6 +4105,8 @@ const_tbl_update(struct autoload_const *ac, int autoload_force)
40954105
rb_const_flag_t visibility = ac->flag;
40964106
rb_const_entry_t *ce;
40974107

4108+
ASSERT_vm_locking();
4109+
40984110
if (rb_id_table_lookup(tbl, id, &value)) {
40994111
ce = (rb_const_entry_t *)value;
41004112
if (UNDEF_P(ce->value)) {
@@ -4141,8 +4153,8 @@ const_tbl_update(struct autoload_const *ac, int autoload_force)
41414153
rb_clear_constant_cache_for_id(id);
41424154

41434155
ce = ZALLOC(rb_const_entry_t);
4144-
rb_id_table_insert(tbl, id, (VALUE)ce);
41454156
setup_const_entry(ce, klass, val, visibility);
4157+
rb_id_table_insert(tbl, id, (VALUE)ce);
41464158
}
41474159
}
41484160

@@ -4190,29 +4202,31 @@ set_const_visibility(VALUE mod, int argc, const VALUE *argv,
41904202
return;
41914203
}
41924204

4193-
for (i = 0; i < argc; i++) {
4194-
struct autoload_const *ac;
4195-
VALUE val = argv[i];
4196-
id = rb_check_id(&val);
4197-
if (!id) {
4198-
undefined_constant(mod, val);
4199-
}
4200-
if ((ce = rb_const_lookup(mod, id))) {
4201-
ce->flag &= ~mask;
4202-
ce->flag |= flag;
4203-
if (UNDEF_P(ce->value)) {
4204-
struct autoload_data *ele;
4205-
4206-
ele = autoload_data_for_named_constant(mod, id, &ac);
4207-
if (ele) {
4208-
ac->flag &= ~mask;
4209-
ac->flag |= flag;
4205+
RB_VM_LOCKING() {
4206+
for (i = 0; i < argc; i++) {
4207+
struct autoload_const *ac;
4208+
VALUE val = argv[i];
4209+
id = rb_check_id(&val);
4210+
if (!id) {
4211+
undefined_constant(mod, val);
4212+
}
4213+
if ((ce = rb_const_lookup(mod, id, NULL))) {
4214+
ce->flag &= ~mask;
4215+
ce->flag |= flag;
4216+
if (UNDEF_P(ce->value)) {
4217+
struct autoload_data *ele;
4218+
4219+
ele = autoload_data_for_named_constant(mod, id, &ac);
4220+
if (ele) {
4221+
ac->flag &= ~mask;
4222+
ac->flag |= flag;
4223+
}
42104224
}
4225+
rb_clear_constant_cache_for_id(id);
4226+
}
4227+
else {
4228+
undefined_constant(mod, ID2SYM(id));
42114229
}
4212-
rb_clear_constant_cache_for_id(id);
4213-
}
4214-
else {
4215-
undefined_constant(mod, ID2SYM(id));
42164230
}
42174231
}
42184232
}
@@ -4228,10 +4242,12 @@ rb_deprecate_constant(VALUE mod, const char *name)
42284242
if (!(id = rb_check_id_cstr(name, len, NULL))) {
42294243
undefined_constant(mod, rb_fstring_new(name, len));
42304244
}
4231-
if (!(ce = rb_const_lookup(mod, id))) {
4232-
undefined_constant(mod, ID2SYM(id));
4245+
RB_VM_LOCKING() {
4246+
if (!(ce = rb_const_lookup(mod, id, NULL))) {
4247+
undefined_constant(mod, ID2SYM(id));
4248+
}
4249+
ce->flag |= CONST_DEPRECATED;
42334250
}
4234-
ce->flag |= CONST_DEPRECATED;
42354251
}
42364252

42374253
/*
@@ -4788,23 +4804,30 @@ rb_fields_tbl_copy(VALUE dst, VALUE src)
47884804
}
47894805

47904806
static rb_const_entry_t *
4791-
const_lookup(struct rb_id_table *tbl, ID id)
4807+
const_lookup(struct rb_id_table *tbl, ID id, rb_const_entry_t *entry_out)
47924808
{
47934809
if (tbl) {
4810+
rb_const_entry_t *ce;
47944811
VALUE val;
47954812
bool r;
47964813
RB_VM_LOCKING() {
47974814
r = rb_id_table_lookup(tbl, id, &val);
4815+
if (r) {
4816+
ce = (rb_const_entry_t*)val;
4817+
if (entry_out) *entry_out = *ce;
4818+
}
47984819
}
47994820

4800-
if (r) return (rb_const_entry_t *)val;
4821+
if (r) {
4822+
return ce;
4823+
}
48014824
}
48024825
return NULL;
48034826
}
48044827

48054828
rb_const_entry_t *
4806-
rb_const_lookup(VALUE klass, ID id)
4829+
rb_const_lookup(VALUE klass, ID id, rb_const_entry_t *entry_out)
48074830
{
4808-
return const_lookup(RCLASS_CONST_TBL(klass), id);
4831+
return const_lookup(RCLASS_CONST_TBL(klass), id, entry_out);
48094832
}
48104833

‎vm_insnhelper.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,10 +1092,11 @@ vm_get_ev_const(rb_execution_context_t *ec, VALUE orig_klass, ID id, bool allow_
10921092
if (!NIL_P(klass)) {
10931093
VALUE av, am = 0;
10941094
rb_const_entry_t *ce;
1095+
rb_const_entry_t ce_out = {0};
10951096
search_continue:
1096-
if ((ce = rb_const_lookup(klass, id))) {
1097-
rb_const_warn_if_deprecated(ce, klass, id);
1098-
val = ce->value;
1097+
if ((ce = rb_const_lookup(klass, id, &ce_out))) {
1098+
rb_const_warn_if_deprecated(&ce_out, klass, id);
1099+
val = ce_out.value;
10991100
if (UNDEF_P(val)) {
11001101
if (am == klass) break;
11011102
am = klass;

0 commit comments

Comments
 (0)