ruby-changes:71091
From: Aaron <ko1@a...>
Date: Sat, 5 Feb 2022 07:36:19 +0900 (JST)
Subject: [ruby-changes:71091] 2a76440fac (master): [Bug #18501] Fire write barrier after hash has been written
https://git.ruby-lang.org/ruby.git/commit/?id=2a76440fac From 2a76440fac62bb0f6e53ccada07caf4b47b78cf9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson <tenderlove@r...> Date: Fri, 28 Jan 2022 10:06:02 -0800 Subject: [Bug #18501] Fire write barrier after hash has been written Before this change the write barrier was executed before the key and value were actually reachable via the Hash. This could cause inconsistencies in object coloration which would lead to accidental collection of dup'd keys. Example: 1. Object O is grey, Object P is white. 2. Write barrier fires O -> P 3. Write barrier does nothing 4. Malloc happens, which starts GC 5. GC colors O black 6. P is written in to O (now we have O -> P reference) 7. P is now accidentally treated as garbage --- hash.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/hash.c b/hash.c index f032ef642a..06b4652cde 100644 --- a/hash.c +++ b/hash.c @@ -1673,6 +1673,8 @@ struct update_arg { https://github.com/ruby/ruby/blob/trunk/hash.c#L1673 st_data_t arg; st_update_callback_func *func; VALUE hash; + VALUE key; + VALUE value; }; typedef int (*tbl_update_func)(st_data_t *, st_data_t *, st_data_t, int); @@ -1705,11 +1707,11 @@ tbl_update_modify(st_data_t *key, st_data_t *val, st_data_t arg, int existing) https://github.com/ruby/ruby/blob/trunk/hash.c#L1707 default: break; case ST_CONTINUE: - if (!existing || *key != old_key || *val != old_value) + if (!existing || *key != old_key || *val != old_value) { rb_hash_modify(hash); - /* write barrier */ - RB_OBJ_WRITTEN(hash, Qundef, *key); - RB_OBJ_WRITTEN(hash, Qundef, *val); + p->key = *key; + p->value = *val; + } break; case ST_DELETE: if (existing) @@ -1727,9 +1729,17 @@ tbl_update(VALUE hash, VALUE key, tbl_update_func func, st_data_t optional_arg) https://github.com/ruby/ruby/blob/trunk/hash.c#L1729 .arg = optional_arg, .func = func, .hash = hash, + .key = key, + .value = (VALUE)optional_arg, }; - return rb_hash_stlike_update(hash, key, tbl_update_modify, (st_data_t)&arg); + int ret = rb_hash_stlike_update(hash, key, tbl_update_modify, (st_data_t)&arg); + + /* write barrier */ + RB_OBJ_WRITTEN(hash, Qundef, arg.key); + RB_OBJ_WRITTEN(hash, Qundef, arg.value); + + return ret; } #define UPDATE_CALLBACK(iter_lev, func) ((iter_lev) > 0 ? func##_noinsert : func##_insert) -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/