danielpclark / rutie

“The Tie Between Ruby and Rust.”
MIT License
939 stars 62 forks source link

Is Rutie#init necessary? #142

Open ball-hayden opened 3 years ago

ball-hayden commented 3 years ago

Thank you for the project, and apologies for the Q&A/Discussion issue. Rutie's really interesting, and has helped myself and @saty9 get a Ruby/Rust integration going pretty quickly.

An issue we've come across is that Rails becomes very upset when loading a gem that has a call out to Rutie#init (specifically, we get Bus Errors). I suspect the issue is with Spring's forking, but it could also be an issue with Zeitwerk, or some of Rails' other loading "magic".

This has led us to investigate a bit further about Ruby's initialisation of native extensions. What we've found is that Ruby will call an initialisation function when requireing a shared library, removing the need for Rutie#init [source].

For example, we can adapt the example in https://github.com/danielpclark/rutie/tree/master/examples/rutie_ruby_example slightly and get the correct behaviour:

--- a/examples/rutie_ruby_example/Rakefile
+++ b/examples/rutie_ruby_example/Rakefile
@@ -18,6 +18,8 @@ task :build_lib do
   else
     sh 'cargo build --release'
   end
+
+  sh "mv target/release/librutie_ruby_example.#{RbConfig::CONFIG['SOEXT']} lib/rutie_ruby_example/rutie_ruby_example_ext.#{RbConfig::CONFIG['DLEXT']}"
 end

 desc 'bundle install'
diff --git a/examples/rutie_ruby_example/lib/rutie_ruby_example.rb b/examples/rutie_ruby_example/lib/rutie_ruby_example.rb
index e0483a0..cd73617 100644
--- a/examples/rutie_ruby_example/lib/rutie_ruby_example.rb
+++ b/examples/rutie_ruby_example/lib/rutie_ruby_example.rb
@@ -1,6 +1,6 @@
 require 'rutie_ruby_example/version'
-require 'rutie'
+require 'rutie_ruby_example/rutie_ruby_example_ext'
+

 module RutieRubyExample
-  Rutie.new(:rutie_ruby_example).init 'Init_rutie_ruby_example', __dir__
 end
diff --git a/examples/rutie_ruby_example/src/lib.rs b/examples/rutie_ruby_example/src/lib.rs
index 7f79ca8..d6d16e0 100644
--- a/examples/rutie_ruby_example/src/lib.rs
+++ b/examples/rutie_ruby_example/src/lib.rs
@@ -26,7 +26,7 @@ methods!(

 #[allow(non_snake_case)]
 #[no_mangle]
-pub extern "C" fn Init_rutie_ruby_example() {
+pub extern "C" fn Init_rutie_ruby_example_ext() {
     Class::new("RutieExample", None).define(|klass| {
         klass.def_self("reverse", pub_reverse);
     });
cd examples/rutie_ruby_example
bundle exec rake
<snip>
RutieRubyExampleTest
 PASS (0.00s) :: test_it_reverses

Finished in 0.00085s
1 tests, 1 assertions, 0 failures, 0 errors, 0 skips

This seems to play more nicely with Rails. My question is: is this safe, or are we missing something?

I'm happy to open a PR if it is safe and you'd like to adopt this pattern.

danielpclark commented 3 years ago

I'm not aware of any issues in doing it that way as long as require will call it. As long as it is initialized I believe that's all it needs.

milgner commented 2 years ago

Just saw that this; might be related to #151 @ball-hayden did you encounter any issues with MRI crashes in the context of Spring reloading?

ball-hayden commented 2 years ago

@milgner this definitely looks related.

After making the changes above (and dropping Rutie#init calls) we've been able to use Rutie with Spring.

We didn't have any further issues - we've been running quite happily in production since mid-April 2021.

milgner commented 2 years ago

@ball-hayden I just gave it a try and it seems to have resolved the problem, too! Many thanks, really great tip! I just had to require_relative the .so file and everything worked as expected.