Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ruby] Backport "ruby: register grpc_rb_sStatus as a global variable (#36125)" to 1.62.x #36508

Open
wants to merge 1 commit into
base: v1.62.x
Choose a base branch
from

Conversation

apolcyn
Copy link
Contributor

@apolcyn apolcyn commented May 2, 2024

Ref: https://bugs.ruby-lang.org/issues/20311

C global variable that contain references to Ruby object MUST be declared to the Ruby GC to prevent these objects from being collected or moved.

There are a few exceptions to that, such as classes defined using the C APIs such as rb_define_class etc.

Up to Ruby 3.4 however, there was a bug that caused classes created from Ruby with the Struct.new("Name") API to also be marked as immortal and immovable.

GRPC has been relying on this bug, which I fixed in Ruby 3.4, and now GRPC is crashing when Struct::Status is moved around by the GC.

-- C level backtrace information -------------------------------------------
ruby(rb_print_backtrace+0x14) [0x5577db219d41] ruby-3.4.0/vm_dump.c:820
ruby(rb_vm_bugreport) ruby-3.4.0/vm_dump.c:1151
ruby(rb_bug_for_fatal_signal+0xfc) [0x5577db3cc60c] ruby-3.4.0/error.c:1066
ruby(sigsegv+0x4d) [0x5577db16358d] ruby-3.4.0/signal.c:926
libc.so.6(0x7f5bacd32520) [0x7f5bacd32520]
ruby(rb_class_superclass+0x32) [0x5577db0a9152] ruby-3.4.0/object.c:2239
ruby(struct_ivar_get+0x2a) [0x5577db194e02] ruby-3.4.0/struct.c:49
ruby(struct_ivar_get) ruby-3.4.0/struct.c:40
ruby(num_members) ruby-3.4.0/struct.c:705
ruby(rb_struct_new+0x56) [0x5577db19a9d6] ruby-3.4.0/struct.c:848
lib/grpc/grpc_c.so(grpc_run_batch_stack_build_result+0xe6) [0x7f5b84bb6b96] /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:780
lib/grpc/grpc_c.so(grpc_rb_call_run_batch_try) /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:839
ruby(rb_ensure+0x10c) [0x5577db007d3c] ruby-3.4.0/eval.c:1000
/tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/lib/grpc/grpc_c.so(grpc_rb_call_run_batch+0xca) [0x7f5b84bb595a] /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:893
ruby(vm_call_cfunc_with_frame_+0x117) [0x5577db1f4c77] ruby-3.4.0/vm_insnhelper.c:3524

cc @apolcyn @peterzhu2118 @k0kubun

Closes #36125

COPYBARA_INTEGRATE_REVIEW=#36125 from Shopify:fix-ruby-3.4-compat 7a12759 PiperOrigin-RevId: 616152904

@apolcyn apolcyn added lang/ruby release notes: yes Indicates if PR needs to be in release notes labels May 2, 2024
@k0kubun
Copy link

k0kubun commented May 2, 2024

Could you remove my GitHub username mention from the commit message? I got multiple notifications from the original commit in fork repositories, and it'd be nice if it doesn't happen again with backports.

Ref: https://bugs.ruby-lang.org/issues/20311

C global variable that contain references to Ruby object MUST be declared to the Ruby GC to prevent these objects from being collected or moved.

There are a few exceptions to that, such as classes defined using the C APIs such as `rb_define_class` etc.

Up to Ruby 3.4 however, there was a bug that caused classes created from Ruby with the `Struct.new("Name")` API to also be marked as immortal and immovable.

GRPC has been relying on this bug, which I fixed in Ruby 3.4, and now GRPC is crashing when `Struct::Status` is moved around by the GC.

```
-- C level backtrace information -------------------------------------------
ruby(rb_print_backtrace+0x14) [0x5577db219d41] ruby-3.4.0/vm_dump.c:820
ruby(rb_vm_bugreport) ruby-3.4.0/vm_dump.c:1151
ruby(rb_bug_for_fatal_signal+0xfc) [0x5577db3cc60c] ruby-3.4.0/error.c:1066
ruby(sigsegv+0x4d) [0x5577db16358d] ruby-3.4.0/signal.c:926
libc.so.6(0x7f5bacd32520) [0x7f5bacd32520]
ruby(rb_class_superclass+0x32) [0x5577db0a9152] ruby-3.4.0/object.c:2239
ruby(struct_ivar_get+0x2a) [0x5577db194e02] ruby-3.4.0/struct.c:49
ruby(struct_ivar_get) ruby-3.4.0/struct.c:40
ruby(num_members) ruby-3.4.0/struct.c:705
ruby(rb_struct_new+0x56) [0x5577db19a9d6] ruby-3.4.0/struct.c:848
lib/grpc/grpc_c.so(grpc_run_batch_stack_build_result+0xe6) [0x7f5b84bb6b96] /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:780
lib/grpc/grpc_c.so(grpc_rb_call_run_batch_try) /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:839
ruby(rb_ensure+0x10c) [0x5577db007d3c] ruby-3.4.0/eval.c:1000
/tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/lib/grpc/grpc_c.so(grpc_rb_call_run_batch+0xca) [0x7f5b84bb595a] /tmp/bundle/ruby/3.4.0+0/bundler/gems/grpc-5ed33ee673e3/src/ruby/ext/grpc/rb_call.c:893
ruby(vm_call_cfunc_with_frame_+0x117) [0x5577db1f4c77] ruby-3.4.0/vm_insnhelper.c:3524
```

cc @apolcyn @peterzhu2118

Closes grpc#36125

COPYBARA_INTEGRATE_REVIEW=grpc#36125 from Shopify:fix-ruby-3.4-compat 7a12759
PiperOrigin-RevId: 616152904
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/ruby release notes: yes Indicates if PR needs to be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants