Skip to content

Commit f0d6fcb

Browse files
committed
Wrap secondary map mutations in a mutex, to avoid mutation races.
1 parent b75a49f commit f0d6fcb

File tree

1 file changed

+10
-0
lines changed

1 file changed

+10
-0
lines changed

ruby/ext/google/protobuf_c/protobuf.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,12 @@ void Arena_register(VALUE module) {
251251
// The object is used only for its identity; it does not contain any data.
252252
VALUE secondary_map = Qnil;
253253

254+
// Mutations to the map are under a mutex, because SeconaryMap_MaybeGC()
255+
// iterates over the map which cannot happen in parallel with insertions, or
256+
// Ruby will throw:
257+
// can't add a new key into hash during iteration (RuntimeError)
258+
VALUE secondary_map_mutex = Qnil;
259+
254260
// Lambda that will GC entries from the secondary map that are no longer present
255261
// in the primary map.
256262
VALUE gc_secondary_map = Qnil;
@@ -260,11 +266,13 @@ extern VALUE weak_obj_cache;
260266
static void SecondaryMap_Init() {
261267
rb_gc_register_address(&secondary_map);
262268
rb_gc_register_address(&gc_secondary_map);
269+
rb_gc_register_address(&secondary_map_mutex);
263270
secondary_map = rb_hash_new();
264271
gc_secondary_map = rb_eval_string(
265272
"->(secondary, weak) {\n"
266273
" secondary.delete_if { |k, v| !weak.key?(v) }\n"
267274
"}\n");
275+
secondary_map_mutex = rb_mutex_new();
268276
}
269277

270278
static void SecondaryMap_MaybeGC() {
@@ -280,9 +288,11 @@ static void SecondaryMap_MaybeGC() {
280288
static VALUE SecondaryMap_Get(VALUE key) {
281289
VALUE ret = rb_hash_lookup(secondary_map, key);
282290
if (ret == Qnil) {
291+
rb_mutex_lock(secondary_map_mutex);
283292
SecondaryMap_MaybeGC();
284293
ret = rb_eval_string("Object.new");
285294
rb_hash_aset(secondary_map, key, ret);
295+
rb_mutex_unlock(secondary_map_mutex);
286296
}
287297
return ret;
288298
}

0 commit comments

Comments
 (0)