Skip to content

Commit fcf2c3b

Browse files
Fix write barriers in rb_hash_add_new_element
The write barriers must be run after the st_update callback returns, as the objects are not on the object until then and there may be allocation when there is a new object inserted. This is hard to reproduce, and I haven't seen an actual crash due to it, but it is detected by wbcheck RUBY_GC_LIBRARY=wbcheck WBCHECK_VERIFY_AFTER_WB=1 ./miniruby -e '("a".."zz").uniq.to_a' WBCHECK ERROR: Missed write barrier detected! Parent object: 0x720db01f99c0 (wb_protected: true) rb_obj_info_dump: 0x0000720db01f99c0 T_HASH/[S] 18 Reference counts - snapshot: 32, writebarrier: 2, current: 36, missed: 2 Missing reference to: 0x716db02e3450 rb_obj_info_dump: 0x0000716db02e3450 T_STRING/String len: 1, capa: 15 "q" Missing reference to: 0x716db02e3450 rb_obj_info_dump: 0x0000716db02e3450 T_STRING/String len: 1, capa: 15 "q" A part of why this is hard to reproduce and it's unlikely to crash is that the insertion only rarely allocates. Co-authored-by: Luke Gruber <[email protected]>
1 parent c351c3d commit fcf2c3b

File tree

1 file changed

+15
-14
lines changed

1 file changed

+15
-14
lines changed

hash.c

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5073,10 +5073,8 @@ rb_hash_deconstruct_keys(VALUE hash, VALUE keys)
50735073
static int
50745074
add_new_i(st_data_t *key, st_data_t *val, st_data_t arg, int existing)
50755075
{
5076-
VALUE *args = (VALUE *)arg;
50775076
if (existing) return ST_STOP;
5078-
RB_OBJ_WRITTEN(args[0], Qundef, (VALUE)*key);
5079-
RB_OBJ_WRITE(args[0], (VALUE *)val, args[1]);
5077+
*val = arg;
50805078
return ST_CONTINUE;
50815079
}
50825080

@@ -5088,22 +5086,25 @@ int
50885086
rb_hash_add_new_element(VALUE hash, VALUE key, VALUE val)
50895087
{
50905088
st_table *tbl;
5091-
int ret = 0;
5092-
VALUE args[2];
5093-
args[0] = hash;
5094-
args[1] = val;
5089+
int ret = -1;
50955090

50965091
if (RHASH_AR_TABLE_P(hash)) {
5097-
ret = ar_update(hash, (st_data_t)key, add_new_i, (st_data_t)args);
5098-
if (ret != -1) {
5099-
return ret;
5092+
ret = ar_update(hash, (st_data_t)key, add_new_i, (st_data_t)val);
5093+
if (ret == -1) {
5094+
ar_force_convert_table(hash, __FILE__, __LINE__);
51005095
}
5101-
ar_force_convert_table(hash, __FILE__, __LINE__);
51025096
}
51035097

5104-
tbl = RHASH_TBL_RAW(hash);
5105-
return st_update(tbl, (st_data_t)key, add_new_i, (st_data_t)args);
5106-
5098+
if (ret == -1) {
5099+
tbl = RHASH_TBL_RAW(hash);
5100+
ret = st_update(tbl, (st_data_t)key, add_new_i, (st_data_t)val);
5101+
}
5102+
if (!ret) {
5103+
// Newly inserted
5104+
RB_OBJ_WRITTEN(hash, Qundef, key);
5105+
RB_OBJ_WRITTEN(hash, Qundef, val);
5106+
}
5107+
return ret;
51075108
}
51085109

51095110
static st_data_t

0 commit comments

Comments
 (0)