From 0d1f36421ca215ee9809fca29683199f38bee34d Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Fri, 4 Mar 2022 21:51:06 +0000 Subject: [PATCH 1/5] Add ruby-specific upb_alloc using xrealloc/xfree for use in Arena_alloc so Ruby GC is aware of allocated memory. --- ruby/ext/google/protobuf_c/protobuf.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index 4d3e1a51448e..405052749917 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -174,6 +174,7 @@ typedef struct { VALUE pinned_objs; } Arena; + static void Arena_mark(void *data) { Arena *arena = data; rb_gc_mark(arena->pinned_objs); @@ -193,9 +194,20 @@ const rb_data_type_t Arena_type = { .flags = RUBY_TYPED_FREE_IMMEDIATELY, }; +static void* ruby_upb_allocfunc(upb_alloc* alloc, void* ptr, size_t oldsize, size_t size) { + if (size == 0) { + xfree(ptr); + return NULL; + } else { + return xrealloc(ptr, size); + } +} + +upb_alloc ruby_upb_alloc = {&ruby_upb_allocfunc}; + static VALUE Arena_alloc(VALUE klass) { Arena *arena = ALLOC(Arena); - arena->arena = upb_Arena_New(); + arena->arena = upb_Arena_Init(NULL, 0, &ruby_upb_alloc); arena->pinned_objs = Qnil; return TypedData_Wrap_Struct(klass, &Arena_type, arena); } From 87967cc5b4ec91bd9327ea98df0f80acd132ccdc Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Fri, 4 Mar 2022 22:08:35 +0000 Subject: [PATCH 2/5] Re-add accidentally deleted new line --- ruby/ext/google/protobuf_c/protobuf.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index 405052749917..2135cca462c9 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -174,7 +174,6 @@ typedef struct { VALUE pinned_objs; } Arena; - static void Arena_mark(void *data) { Arena *arena = data; rb_gc_mark(arena->pinned_objs); From d45843a5d31fc9269b23f362702075b2517cb793 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Tue, 8 Mar 2022 00:27:06 +0000 Subject: [PATCH 3/5] Add RB_GC_GUARD to ensure ruby does not aggressively garbage collect arena_rb due to lack of references, when memory is still used by file_proto --- ruby/ext/google/protobuf_c/defs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index aaa8b4dc7af0..3bd18e840028 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -159,6 +159,7 @@ VALUE DescriptorPool_add_serialized_file(VALUE _self, rb_raise(cTypeError, "Unable to build file to DescriptorPool: %s", upb_Status_ErrorMessage(&status)); } + RB_GC_GUARD(arena_rb); return get_filedef_obj(_self, filedef); } From f4fed16f00a9f23a2355f65b63281461ad2cbd14 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Tue, 8 Mar 2022 00:30:28 +0000 Subject: [PATCH 4/5] Enable GC.stress in gc_test for ruby 2.7. This was previously disabled due to a ruby GC bug which has since been closed: https://bugs.ruby-lang.org/issues/16807 --- ruby/tests/gc_test.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ruby/tests/gc_test.rb b/ruby/tests/gc_test.rb index d7fecaeea108..e257f0c3daa5 100755 --- a/ruby/tests/gc_test.rb +++ b/ruby/tests/gc_test.rb @@ -4,9 +4,7 @@ $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) old_gc = GC.stress -# Ruby 2.7.0 - 2.7.1 has a GC bug in its parser, so turn off stress for now -# See https://bugs.ruby-lang.org/issues/16807 -GC.stress = 0x01 | 0x04 unless RUBY_VERSION.match?(/^2\.7\./) +GC.stress = 0x01 | 0x04 require 'generated_code_pb' require 'generated_code_proto2_pb' GC.stress = old_gc @@ -94,7 +92,6 @@ def test_generated_msg from = get_msg_proto3 data = A::B::C::TestMessage.encode(from) to = A::B::C::TestMessage.decode(data) - # This doesn't work for proto2 on JRuby because there is a nested required message. # A::B::Proto2::TestMessage has :required_msg which is of type: # A::B::Proto2::TestMessage so there is no way to generate a valid From 79bb0af1bf478fbf416c7b34db1dd8f9e739e04d Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Tue, 8 Mar 2022 00:50:15 +0000 Subject: [PATCH 5/5] Undo previous commit that enabled gc.stress for ruby 2.7. Though the issue is theoretically fixed (probably at a different version), Kokoro uses 2.7.0 and fails --- ruby/tests/gc_test.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ruby/tests/gc_test.rb b/ruby/tests/gc_test.rb index e257f0c3daa5..5d48f464aee5 100755 --- a/ruby/tests/gc_test.rb +++ b/ruby/tests/gc_test.rb @@ -4,7 +4,9 @@ $LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) old_gc = GC.stress -GC.stress = 0x01 | 0x04 +# Ruby 2.7.0 - 2.7.1 has a GC bug in its parser, so turn off stress for now +# See https://bugs.ruby-lang.org/issues/16807 +GC.stress = 0x01 | 0x04 unless RUBY_VERSION.match?(/^2\.7\./) require 'generated_code_pb' require 'generated_code_proto2_pb' GC.stress = old_gc