From cd5f49d0942e19a5854a325941918fca02fdb409 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Tue, 3 Oct 2017 17:28:49 -0700 Subject: Fix ruby segment fault (#3708) * Fix ruby segment fault 1) rb_ary_new cannot be called during allocate function. During allocate fucntion, the containing object hasn't been marked and rb_ary_new may invoke gc to collect containing object. 2) The global map should be marked before allocating it. Otherwise it may be garbage collected. * Add test * Remove commented code * Fix grammer error --- ruby/ext/google/protobuf_c/defs.c | 17 ++++++++++++++--- ruby/ext/google/protobuf_c/protobuf.c | 2 +- ruby/ext/google/protobuf_c/protobuf.h | 5 +---- 3 files changed, 16 insertions(+), 8 deletions(-) (limited to 'ruby/ext/google') diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 845c0225..34d9663a 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -228,7 +228,6 @@ DEFINE_CLASS(Descriptor, "Google::Protobuf::Descriptor"); void Descriptor_mark(void* _self) { Descriptor* self = _self; rb_gc_mark(self->klass); - rb_gc_mark(self->typeclass_references); } void Descriptor_free(void* _self) { @@ -283,7 +282,6 @@ VALUE Descriptor_alloc(VALUE klass) { self->pb_serialize_handlers = NULL; self->json_serialize_handlers = NULL; self->json_serialize_handlers_preserve = NULL; - self->typeclass_references = rb_ary_new(); return ret; } @@ -1635,7 +1633,7 @@ VALUE Builder_alloc(VALUE klass) { Builder* self = ALLOC(Builder); VALUE ret = TypedData_Wrap_Struct( klass, &_Builder_type, self); - self->pending_list = rb_ary_new(); + self->pending_list = Qnil; self->defs = NULL; return ret; } @@ -1645,11 +1643,24 @@ void Builder_register(VALUE module) { rb_define_alloc_func(klass, Builder_alloc); rb_define_method(klass, "add_message", Builder_add_message, 1); rb_define_method(klass, "add_enum", Builder_add_enum, 1); + rb_define_method(klass, "initialize", Builder_initialize, 0); rb_define_method(klass, "finalize_to_pool", Builder_finalize_to_pool, 1); cBuilder = klass; rb_gc_register_address(&cBuilder); } +/* + * call-seq: + * Builder.new(d) => builder + * + * Create a new message builder. + */ +VALUE Builder_initialize(VALUE _self) { + DEFINE_SELF(Builder, self, _self); + self->pending_list = rb_ary_new(); + return Qnil; +} + /* * call-seq: * Builder.add_message(name, &block) diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index 7cde4aec..c7750c44 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -110,6 +110,6 @@ void Init_protobuf_c() { kRubyStringASCIIEncoding = rb_usascii_encoding(); kRubyString8bitEncoding = rb_ascii8bit_encoding(); - upb_def_to_ruby_obj_map = rb_hash_new(); rb_gc_register_address(&upb_def_to_ruby_obj_map); + upb_def_to_ruby_obj_map = rb_hash_new(); } diff --git a/ruby/ext/google/protobuf_c/protobuf.h b/ruby/ext/google/protobuf_c/protobuf.h index f4b110fe..21c7f001 100644 --- a/ruby/ext/google/protobuf_c/protobuf.h +++ b/ruby/ext/google/protobuf_c/protobuf.h @@ -116,10 +116,6 @@ struct Descriptor { const upb_handlers* pb_serialize_handlers; const upb_handlers* json_serialize_handlers; const upb_handlers* json_serialize_handlers_preserve; - // Handlers hold type class references for sub-message fields directly in some - // cases. We need to keep these rooted because they might otherwise be - // collected. - VALUE typeclass_references; }; struct FieldDescriptor { @@ -280,6 +276,7 @@ void Builder_free(void* _self); VALUE Builder_alloc(VALUE klass); void Builder_register(VALUE module); Builder* ruby_to_Builder(VALUE value); +VALUE Builder_initialize(VALUE _self); VALUE Builder_add_message(VALUE _self, VALUE name); VALUE Builder_add_enum(VALUE _self, VALUE name); VALUE Builder_finalize_to_pool(VALUE _self, VALUE pool_rb); -- cgit v1.2.3