Skip to content

Commit be58cd4

Browse files
luke-gruberbyroot
luke-gruber
authored and
byroot
committed
Ractor: lock around global variable get/set
There's a global id_table `rb_global_tbl` that needs a lock (I used VM lock). In the future, we might use a lock-free rb_id_table if we create such a data structure. Reproduction script that might crash or behave strangely: ```ruby 100.times do Ractor.new do 1_000_000.times do $stderr $stdout $stdin $VERBOSE $stderr $stdout $stdin $VERBOSE $stderr $stdout $stdin $VERBOSE end end end $myglobal0 = nil; $myglobal1 = nil; # ... vim macros to the rescue $myglobal100000 = nil; ```
1 parent c3d91eb commit be58cd4

File tree

2 files changed

+83
-72
lines changed

2 files changed

+83
-72
lines changed

‎bootstraptest/test_ractor.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,8 @@ def check obj1
588588
r.value[:frozen]
589589
}
590590

591-
# Access to global-variables are prohibited
592-
assert_equal 'can not access global variables $gv from non-main Ractors', %q{
591+
# Access to global-variables are prohibited (read)
592+
assert_equal 'can not access global variable $gv from non-main Ractor', %q{
593593
$gv = 1
594594
r = Ractor.new do
595595
$gv
@@ -602,8 +602,8 @@ def check obj1
602602
end
603603
}
604604

605-
# Access to global-variables are prohibited
606-
assert_equal 'can not access global variables $gv from non-main Ractors', %q{
605+
# Access to global-variables are prohibited (write)
606+
assert_equal 'can not access global variable $gv from non-main Ractor', %q{
607607
r = Ractor.new do
608608
$gv = 1
609609
end

‎variable.c‎

Lines changed: 79 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -584,16 +584,18 @@ rb_find_global_entry(ID id)
584584
struct rb_global_entry *entry;
585585
VALUE data;
586586

587-
if (!rb_id_table_lookup(rb_global_tbl, id, &data)) {
588-
entry = NULL;
589-
}
590-
else {
591-
entry = (struct rb_global_entry *)data;
592-
RUBY_ASSERT(entry != NULL);
587+
RB_VM_LOCKING() {
588+
if (!rb_id_table_lookup(rb_global_tbl, id, &data)) {
589+
entry = NULL;
590+
}
591+
else {
592+
entry = (struct rb_global_entry *)data;
593+
RUBY_ASSERT(entry != NULL);
594+
}
593595
}
594596

595597
if (UNLIKELY(!rb_ractor_main_p()) && (!entry || !entry->ractor_local)) {
596-
rb_raise(rb_eRactorIsolationError, "can not access global variables %s from non-main Ractors", rb_id2name(id));
598+
rb_raise(rb_eRactorIsolationError, "can not access global variable %s from non-main Ractor", rb_id2name(id));
597599
}
598600

599601
return entry;
@@ -621,25 +623,28 @@ rb_gvar_undef_compactor(void *var)
621623
static struct rb_global_entry*
622624
rb_global_entry(ID id)
623625
{
624-
struct rb_global_entry *entry = rb_find_global_entry(id);
625-
if (!entry) {
626-
struct rb_global_variable *var;
627-
entry = ALLOC(struct rb_global_entry);
628-
var = ALLOC(struct rb_global_variable);
629-
entry->id = id;
630-
entry->var = var;
631-
entry->ractor_local = false;
632-
var->counter = 1;
633-
var->data = 0;
634-
var->getter = rb_gvar_undef_getter;
635-
var->setter = rb_gvar_undef_setter;
636-
var->marker = rb_gvar_undef_marker;
637-
var->compactor = rb_gvar_undef_compactor;
638-
639-
var->block_trace = 0;
640-
var->trace = 0;
641-
var->namespace_ready = false;
642-
rb_id_table_insert(rb_global_tbl, id, (VALUE)entry);
626+
struct rb_global_entry *entry;
627+
RB_VM_LOCKING() {
628+
entry = rb_find_global_entry(id);
629+
if (!entry) {
630+
struct rb_global_variable *var;
631+
entry = ALLOC(struct rb_global_entry);
632+
var = ALLOC(struct rb_global_variable);
633+
entry->id = id;
634+
entry->var = var;
635+
entry->ractor_local = false;
636+
var->counter = 1;
637+
var->data = 0;
638+
var->getter = rb_gvar_undef_getter;
639+
var->setter = rb_gvar_undef_setter;
640+
var->marker = rb_gvar_undef_marker;
641+
var->compactor = rb_gvar_undef_compactor;
642+
643+
var->block_trace = 0;
644+
var->trace = 0;
645+
var->namespace_ready = false;
646+
rb_id_table_insert(rb_global_tbl, id, (VALUE)entry);
647+
}
643648
}
644649
return entry;
645650
}
@@ -1003,15 +1008,17 @@ rb_gvar_set(ID id, VALUE val)
10031008
struct rb_global_entry *entry;
10041009
const rb_namespace_t *ns = rb_current_namespace();
10051010

1006-
entry = rb_global_entry(id);
1011+
RB_VM_LOCKING() {
1012+
entry = rb_global_entry(id);
10071013

1008-
if (USE_NAMESPACE_GVAR_TBL(ns, entry)) {
1009-
rb_hash_aset(ns->gvar_tbl, rb_id2sym(entry->id), val);
1010-
retval = val;
1011-
// TODO: think about trace
1012-
}
1013-
else {
1014-
retval = rb_gvar_set_entry(entry, val);
1014+
if (USE_NAMESPACE_GVAR_TBL(ns, entry)) {
1015+
rb_hash_aset(ns->gvar_tbl, rb_id2sym(entry->id), val);
1016+
retval = val;
1017+
// TODO: think about trace
1018+
}
1019+
else {
1020+
retval = rb_gvar_set_entry(entry, val);
1021+
}
10151022
}
10161023
return retval;
10171024
}
@@ -1026,27 +1033,30 @@ VALUE
10261033
rb_gvar_get(ID id)
10271034
{
10281035
VALUE retval, gvars, key;
1029-
struct rb_global_entry *entry = rb_global_entry(id);
1030-
struct rb_global_variable *var = entry->var;
10311036
const rb_namespace_t *ns = rb_current_namespace();
1032-
1033-
if (USE_NAMESPACE_GVAR_TBL(ns, entry)) {
1034-
gvars = ns->gvar_tbl;
1035-
key = rb_id2sym(entry->id);
1036-
if (RTEST(rb_hash_has_key(gvars, key))) { // this gvar is already cached
1037-
retval = rb_hash_aref(gvars, key);
1037+
// TODO: use lock-free rb_id_table when it's available for use (doesn't yet exist)
1038+
RB_VM_LOCKING() {
1039+
struct rb_global_entry *entry = rb_global_entry(id);
1040+
struct rb_global_variable *var = entry->var;
1041+
1042+
if (USE_NAMESPACE_GVAR_TBL(ns, entry)) {
1043+
gvars = ns->gvar_tbl;
1044+
key = rb_id2sym(entry->id);
1045+
if (RTEST(rb_hash_has_key(gvars, key))) { // this gvar is already cached
1046+
retval = rb_hash_aref(gvars, key);
1047+
}
1048+
else {
1049+
retval = (*var->getter)(entry->id, var->data);
1050+
if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) {
1051+
retval = rb_funcall(retval, rb_intern("clone"), 0);
1052+
}
1053+
rb_hash_aset(gvars, key, retval);
1054+
}
10381055
}
10391056
else {
10401057
retval = (*var->getter)(entry->id, var->data);
1041-
if (rb_obj_respond_to(retval, rb_intern("clone"), 1)) {
1042-
retval = rb_funcall(retval, rb_intern("clone"), 0);
1043-
}
1044-
rb_hash_aset(gvars, key, retval);
10451058
}
10461059
}
1047-
else {
1048-
retval = (*var->getter)(entry->id, var->data);
1049-
}
10501060
return retval;
10511061
}
10521062

@@ -1128,35 +1138,36 @@ rb_f_global_variables(void)
11281138
void
11291139
rb_alias_variable(ID name1, ID name2)
11301140
{
1131-
struct rb_global_entry *entry1, *entry2;
1141+
struct rb_global_entry *entry1 = NULL, *entry2;
11321142
VALUE data1;
11331143
struct rb_id_table *gtbl = rb_global_tbl;
11341144

11351145
if (!rb_ractor_main_p()) {
11361146
rb_raise(rb_eRactorIsolationError, "can not access global variables from non-main Ractors");
11371147
}
11381148

1139-
entry2 = rb_global_entry(name2);
1140-
if (!rb_id_table_lookup(gtbl, name1, &data1)) {
1141-
entry1 = ALLOC(struct rb_global_entry);
1142-
entry1->id = name1;
1143-
rb_id_table_insert(gtbl, name1, (VALUE)entry1);
1144-
}
1145-
else if ((entry1 = (struct rb_global_entry *)data1)->var != entry2->var) {
1146-
struct rb_global_variable *var = entry1->var;
1147-
if (var->block_trace) {
1148-
rb_raise(rb_eRuntimeError, "can't alias in tracer");
1149+
RB_VM_LOCKING() {
1150+
entry2 = rb_global_entry(name2);
1151+
if (!rb_id_table_lookup(gtbl, name1, &data1)) {
1152+
entry1 = ALLOC(struct rb_global_entry);
1153+
entry1->id = name1;
1154+
rb_id_table_insert(gtbl, name1, (VALUE)entry1);
1155+
}
1156+
else if ((entry1 = (struct rb_global_entry *)data1)->var != entry2->var) {
1157+
struct rb_global_variable *var = entry1->var;
1158+
if (var->block_trace) {
1159+
rb_raise(rb_eRuntimeError, "can't alias in tracer");
1160+
}
1161+
var->counter--;
1162+
if (var->counter == 0) {
1163+
free_global_variable(var);
1164+
}
11491165
}
1150-
var->counter--;
1151-
if (var->counter == 0) {
1152-
free_global_variable(var);
1166+
if (entry1) {
1167+
entry2->var->counter++;
1168+
entry1->var = entry2->var;
11531169
}
11541170
}
1155-
else {
1156-
return;
1157-
}
1158-
entry2->var->counter++;
1159-
entry1->var = entry2->var;
11601171
}
11611172

11621173
static void

0 commit comments

Comments
 (0)