From b245551a61ce02a2d1b629de0351ee106321aeac Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 20 Aug 2019 14:08:31 -0700 Subject: [PATCH] Fix for race in lazy initialization of handlers. This fixes https://github.com/protocolbuffers/protobuf/issues/6532. --- ruby/ext/google/protobuf_c/defs.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 77ef5e554079..f53c67ae6d07 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -2228,6 +2228,27 @@ static VALUE get_def_obj(VALUE _descriptor_pool, const void* ptr, VALUE klass) { VALUE args[3] = { c_only_cookie, _descriptor_pool, key }; def = rb_class_new_instance(3, args, klass); rb_hash_aset(descriptor_pool->def_to_descriptor, key, def); + + // For message defs, we now eagerly get/create descriptors for all + // submessages. We will need these anyway to parse or serialize this + // message type. But more importantly, we must do this now so that + // add_handlers_for_message() (which calls get_msgdef_obj()) does *not* + // need to create a Ruby object or insert into a Ruby Hash. We need to + // avoid triggering GC, which can switch Ruby threads and re-enter our + // C extension from a different thread. This wreaks havoc on our state + // if we were in the middle of building handlers. + if (klass == cDescriptor) { + const upb_msgdef *m = ptr; + upb_msg_field_iter it; + for (upb_msg_field_begin(&it, m); + !upb_msg_field_done(&it); + upb_msg_field_next(&it)) { + const upb_fielddef* f = upb_msg_iter_field(&it); + if (upb_fielddef_issubmsg(f)) { + get_msgdef_obj(_descriptor_pool, upb_fielddef_msgsubdef(f)); + } + } + } } return def;