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

Set Ruby C calls as Ractor-safe #19321

Open
cretz opened this issue Nov 19, 2024 · 4 comments
Open

Set Ruby C calls as Ractor-safe #19321

cretz opened this issue Nov 19, 2024 · 4 comments
Labels

Comments

@cretz
Copy link

cretz commented Nov 19, 2024

What language does this apply to?

Ruby

Describe the problem you are trying to solve.

Be able to use Protobuf objects inside of Ruby Ractors.

Describe the solution you'd like

Today, you get things like:

`method_missing': ractor unsafe method called from not main ractor

This is because by default every C extension is considered ractor-unsafe at definition time by default (see ruby/ruby#3824), so Google::Protobuf::AbstractMethod#method_missing as a C method is considered unsafe. They are considered unsafe by default because Ractors are parallel without GVL/GIL and therefore the C extensions must be thread safe within themselves. It appears Protobuf implementation is (not to be confused with general thread safety), so rb_ext_ractor_safe(true); can be called at the top of

__attribute__((visibility("default"))) void Init_protobuf_c() {
. This is available in all non-EOL Ruby versions.

Also make a test for creation and use of Protobuf objects inside Ractors (bonus points for making the objects RUBY_TYPED_FROZEN_SHAREABLE and testing they can be made shareable). That way the FFI-based implementation can be confirmed Ractor-safe as well.

Also, assuming Ractor support is easily added for both implementations, would a PR to support this be welcomed? And backported to the 3.x series (but if not that's ok)?

@cretz cretz added the untriaged auto added to all issues by default when created. label Nov 19, 2024
@acozzette acozzette added the ruby label Nov 22, 2024
@haberman
Copy link
Member

I'm not sure that our code is Ractor-safe at the moment. We use a global object cache to map C pointers to Ruby objects:

VALUE weak_obj_cache = Qnil;

We would need to remove this, and carefully audit all of our other code, before we could claim to be Ractor-safe.

@acozzette acozzette removed the untriaged auto added to all issues by default when created. label Nov 22, 2024
@cretz
Copy link
Author

cretz commented Nov 22, 2024

Eek, true, maybe same issue with the cArena above that.

I have a new project with a use case to use Protobuf objects in Ractors. Can you give some insight into the expected future of the ffi based google-protobuf implementation in Ruby? Specifically, are there near-term plans to make it the default (and is it Ractor safe)? If not, knowing that the C-extension approach would require some refactoring, is it safe to say that this library is not going to be safe for Ractor use for a long time?

@haberman
Copy link
Member

Are ffi-based libraries necessarily Ractor-safe? What if they are accessing shared state? Our ffi implementation also uses a global object cache: https://github.com/protocolbuffers/protobuf/blob/main/ruby/lib/google/protobuf/ffi/object_cache.rb#L28

I think it would take some careful work to be Ractor-safe. It would probably be difficult for us to prioritize this work. How widely-used is Ractor? I believe this is the first feature request we've gotten about it.

@cretz
Copy link
Author

cretz commented Nov 22, 2024

Are ffi-based libraries necessarily Ractor-safe? What if they are accessing shared state

I admit I am not familiar enough with ffi to know, though I did find it can work at https://github.com/ffi/ffi/wiki/Ractors. It does seem like the OBJECT_CACHE would not be able to be shared since it would have to be frozen I suspect. Also unsure if/when ffi would become the default Ruby impl for this library.

How widely-used is Ractor?

Not very widely at all, and it's technically still experimental (though it has been around for many years). It does not surprise me that this is the first feature request for it. Not being able to prioritize Ractor support is totally justifiable. Can leave this issue open for if/when that occurs and I may be able to look into alternatives for now (for me, I will probably want to avoid the pure Ruby library, but since I'm using a Rust C extension in my project anyways, I may be able to expose prost-backed Ruby objects marked frozen-shareable).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants