[Ruby] Composed CallCredentials keep a reference to their source#41782
Closed
etiennebarrie wants to merge 2 commits into
Closed
[Ruby] Composed CallCredentials keep a reference to their source#41782etiennebarrie wants to merge 2 commits into
etiennebarrie wants to merge 2 commits into
Conversation
When building composed CallCredentials, we not only need to keep a reference to all the CallCredentials passed as arguments, but also `self`.
Member
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a garbage collection issue with composed CallCredentials by ensuring the self object is also marked to be kept alive. The added regression test effectively reproduces the problem and verifies the fix. The changes look good.
asheshvidyut
approved these changes
Mar 8, 2026
Member
There was a problem hiding this comment.
Thanks for the PR @etiennebarrie
LGTM 🚀
I have tested and verified this PR running the following commands.
cd grpc
bundle exec rake compile
cd grpc/src/ruby
bundle exec rspec spec/client_server_spec.rb
Zgoda91
pushed a commit
to Zgoda91/grpc
that referenced
this pull request
Mar 22, 2026
…c#41782) Fix grpc#41781 Looking at the backtrace, we can see it happens when the Proc generating credentials metadata gets called here: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L88-L89 The proc is stored here to be marked by the Ruby GC on the CallCredentials object itself: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L292 It's possible to compose CallCredentials. The `CallCredentials#compose` method does keep marking the arguments passed to it by collecting them: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L308-L311 and passing them in the wrapping struct to be stored and later marked: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L319 However it does not keep `self` to mark, which means that if you have two call credentials, combine them, keep the result but get rid of the two original objects, only the second one and its Proc will be marked, but the Proc of the first one can be garbage collected and then cause the crash we're seeing. The test reproduces the issue, crashes on main, but passes with the fix. Closes grpc#41782 COPYBARA_INTEGRATE_REVIEW=grpc#41782 from Shopify:ruby-composed-callcredentials-keep-a-reference-to-their-source 4850fc1 PiperOrigin-RevId: 882286618
asheshvidyut
pushed a commit
to asheshvidyut/grpc
that referenced
this pull request
Mar 26, 2026
…c#41782) Fix grpc#41781 Looking at the backtrace, we can see it happens when the Proc generating credentials metadata gets called here: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L88-L89 The proc is stored here to be marked by the Ruby GC on the CallCredentials object itself: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L292 It's possible to compose CallCredentials. The `CallCredentials#compose` method does keep marking the arguments passed to it by collecting them: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L308-L311 and passing them in the wrapping struct to be stored and later marked: https://github.com/grpc/grpc/blob/093085cc925e0d5aa6e92bc29e917f9bdc00add2/src/ruby/ext/grpc/rb_call_credentials.c#L319 However it does not keep `self` to mark, which means that if you have two call credentials, combine them, keep the result but get rid of the two original objects, only the second one and its Proc will be marked, but the Proc of the first one can be garbage collected and then cause the crash we're seeing. The test reproduces the issue, crashes on main, but passes with the fix. Closes grpc#41782 COPYBARA_INTEGRATE_REVIEW=grpc#41782 from Shopify:ruby-composed-callcredentials-keep-a-reference-to-their-source 4850fc1 PiperOrigin-RevId: 882286618
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #41781
Looking at the backtrace, we can see it happens when the Proc generating credentials metadata gets called here:
grpc/src/ruby/ext/grpc/rb_call_credentials.c
Lines 88 to 89 in 093085c
The proc is stored here to be marked by the Ruby GC on the CallCredentials object itself:
grpc/src/ruby/ext/grpc/rb_call_credentials.c
Line 292 in 093085c
It's possible to compose CallCredentials. The
CallCredentials#composemethod does keep marking the arguments passed to it by collecting them:grpc/src/ruby/ext/grpc/rb_call_credentials.c
Lines 308 to 311 in 093085c
and passing them in the wrapping struct to be stored and later marked:
grpc/src/ruby/ext/grpc/rb_call_credentials.c
Line 319 in 093085c
However it does not keep
selfto mark, which means that if you have two call credentials, combine them, keep the result but get rid of the two original objects, only the second one and its Proc will be marked, but the Proc of the first one can be garbage collected and then cause the crash we're seeing.The test reproduces the issue, crashes on main, but passes with the fix.