From efc34c6840a812c07593554fc7c5a66aead989e8 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 14 Jun 2023 16:06:15 +0200 Subject: [PATCH] Ruby: Only add RepeatedField instance to cache once fully initialized Fix: https://github.com/protocolbuffers/protobuf/issues/11968 Since calling a method on WeakMap allow the RubyVM to switch threads, adding a non-fully initialized object into the cache allow another thread from using it. Note: with this fix, the other two thread may create their own wrapper for the same underlying Array. We don't know if it's a big deal or not. Co-Authored-By: Peter Zhu --- ruby/ext/google/protobuf_c/repeated_field.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index bcd5f45afb18..9b8a6860fdae 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -87,7 +87,6 @@ VALUE RepeatedField_GetRubyWrapper(upb_Array* array, TypeInfo type_info, if (val == Qnil) { val = RepeatedField_alloc(cRepeatedField); RepeatedField* self; - ObjectCache_Add(array, val); TypedData_Get_Struct(val, RepeatedField, &RepeatedField_type, self); self->array = array; self->arena = arena; @@ -95,6 +94,7 @@ VALUE RepeatedField_GetRubyWrapper(upb_Array* array, TypeInfo type_info, if (self->type_info.type == kUpb_CType_Message) { self->type_class = Descriptor_DefToClass(type_info.def.msgdef); } + ObjectCache_Add(array, val); } PBRUBY_ASSERT(ruby_to_RepeatedField(val)->type_info.type == type_info.type);