matsadler / magnus

Ruby bindings for Rust. Write Ruby extension gems in Rust, or call Ruby from Rust.
https://docs.rs/magnus/latest/magnus/
MIT License
682 stars 35 forks source link

Panic when use ruby-static feature #54

Open elct9620 opened 1 year ago

elct9620 commented 1 year ago

I am trying to use magnus with godot-rust but got some errors that are hard to track.

The init code in the godot-rust

fn init(handle: InitHandle) {
    let _cleanup = unsafe { embed::init() };
    handle.add_class::<SomeClass>();
}

godot_init!(init); # L27

And I got this error

E 0:00:02.291   <unset>: Panic message: Ruby init code not executable
  <C++ Source>  /Users/[USER]/.cargo/registry/src/github.com-1ecc6299db9ec823/gdnative-core-0.11.3/src/private.rs:199 @ <unset>()

The lib.rs:27 is points to godot_init!(init); and the panic is raised by embed::init() If I remove the ruby-static feature everything works correctly.

Command to build the library on macOS (Ventura 13.1)

RUBY=~/Downloads/ruby-3.2.0/dist/bin/ruby cargo build

the .cargo/config.toml is configured

P.S. if didn't use godot-rust run as a standalone binary instead of dylib, it works correctly.

Other information

nm -gU target/debug/libexample.dylib

0000000000009d20 T _godot_gdnative_init
000000000000a380 T _godot_gdnative_terminate
0000000000009d70 T _godot_nativescript_init

the nm -m target/debug/libexample.dylib can find private external symbols from ruby static library

otool -L target/debug/libexample.dylib

target/debug/libexample.dylib:
        /Users/[USER]/Desktop/Godot/target/debug/deps/libexample.dylib (compatibility version 0.0.0, current version 0.0.0)
        /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60420.60.24)
        /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1953.255.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.0.0)
        /usr/local/opt/gmp/lib/libgmp.10.dylib (compatibility version 15.0.0, current version 15.1.0)
        /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1953.255.0)
matsadler commented 1 year ago

So that error is coming from https://github.com/matsadler/magnus/blob/ddebce7585feb71ec3097297e77d89a506a2971b/src/embed.rs#L79-L81

The embed::init function first calls ruby_setup which is meant to start the Ruby VM and initialise all the libraries, but it seems that as Ruby has grown parts of the initialisation have moved to other places, so calling just ruby_setup doesn't get us a fully functioning Ruby (sorry, I forget what is missing, require maybe? Maybe some other stuff).

So next we call ruby_options with some dummy options (-e '') because some more initialisation happens there. This returns some compiled Ruby instructions or an opaque error value.

That value is passed to ruby_executable_node which tells us if it was an error or not. In your case it was an error, so it panics.

The final step would be to pass the non-error value to ruby_exec_node which would do the last bit of the initialisation.

I do not know why ruby_options would have returned an error value.

I just pushed a commit swapping to ruby_process_options which raises a Ruby exception and might surface a more descriptive error. I think I hadn't used this initially because I wrote the init code before I fully had the exception handling code working.

You could try pointing your Magnus dependency to this git repo and give that a try and see if the error is something actionable.

Another approach would be to add rb-sys to your project and call rb_sys::ruby_setup() instead of magnus::embed::init(). That might get you a Ruby with enough features working for whatever you're trying to do.

elct9620 commented 1 year ago

After updating to use the new commit, the error message is interesting 🤔

E 0:00:01.833   <unset>: Panic message: called `Result::unwrap()` on an `Err` value: Exception(#<LoadError: dlopen(/Users/elct9620/Downloads/ruby-2.7.7/dist/lib/ruby/2.7.0/x86_64-darwin22/monitor.bundle, 0x0009): Symbol not found: _rb_bug
  Referenced from: <46619371-FF6B-337A-B9FF-5231026AA9DB> /Users/elct9620/Downloads/ruby-2.7.7/dist/lib/ruby/2.7.0/x86_64-darwin22/monitor.bundle
  Expected in:     <A0E9C986-5783-33EB-B003-44584D5E8ED1> /Applications/Godot_mono.app/Contents/MacOS/Godot - /Users/elct9620/Downloads/ruby-2.7.7/dist/lib/ruby/2.7.0/x86_64-darwin22/monitor.bundle>)
  <C++ Source>  /Users/elct9620/.cargo/registry/src/github.com-1ecc6299db9ec823/gdnative-core-0.11.3/src/private.rs:197 @ <unset>()

P.S. use rb_sys::ruby-setup() is work correctly for now.

matsadler commented 1 year ago

Oh, I wonder if (despite the slightly different error message) that's the same issue that is described in the Troubleshooting - Issues with static linking section of the readme. Have you given that a go?

elct9620 commented 1 year ago

Yes, I had tested with extra config. The dynamic library grows from 7MB to 37MB, I think it should bundle it.

matsadler commented 1 year ago

I've added embed::setup() to call just ruby_setup().

elct9620 commented 1 year ago

After setting the revision to use 52eeaad and embed::setup() new issues are appearing.

The Godot freezes when I run the game to test, but using rb_sys::setup() doesn't have this issue.

Another confuse status is when I forget to add rb_sys::setup() or embed::setup() I still got Ruby already initialized panic

ianks commented 1 year ago

can you post a small repo with the all the code that shows this so I can take a look?

elct9620 commented 1 year ago

I create a repo that included my test code: https://github.com/elct9620/godot-with-ruby_poc

ianks commented 1 year ago

I noticed in the gdn config that it may be trying to load libruby.so. If you are linking ruby statically, it shouldn’t be necessary. Does Godot expect dylibs?

I ask because having Ruby expects one and only one VM to be setup, and having multiple versions loaded would break assumptions Ruby will make about the location of thread locals, etc

elct9620 commented 1 year ago

Godot expect dylibs because we didn't compile the extension into the game engine. We are creating a dynamic library and using gdn config to define where to load it.

My project is loaded ruby in this way: Godot -> libruby.so (the extension bundled ruby)

P.S. the rust godot SDK does not support "singleton" mode for now. When I start a new process it is panic therefore I guess it breaks the first time.

ianks commented 1 year ago

Still a bit confused on this. It seems like you want both libruby.so and to link ruby statically (which is not possible). Is that right?

ianks commented 1 year ago

Ahh I see, you named to crate "ruby" so I was getting confused. One thing you should add as well is a build.rs, so we can propagate linker flags to your crate:

First, add rb-sys-env to your Cargo.toml:

diff --git i/Cargo.toml w/Cargo.toml
index 46d4e99..582d6c9 100644
--- i/Cargo.toml
+++ w/Cargo.toml
@@ -8,5 +8,8 @@ gdnative = "0.11"
 rb-sys = "0.9.56"
 magnus = { git = "https://github.com/matsadler/magnus.git", rev = "52eeaad", features = ["embed", "ruby-static"] }

+[build-dependencies]
+rb-sys-env = "0.1.2"
+
 [lib]
 crate-type = ["cdylib"]

Then:

// build.rs

use std::error::Error;

fn main() -> Result<(), Box<dyn Error>> {
    let _ = rb_sys_env::activate()?;

    Ok(())
}

This gets us to the point where we have linked up all of the libruby-static symbols:

❯ nm --defined-only /tmp/godot-with-ruby_poc/target/debug/libruby.dylib | grep ' _rb_str_new'
00000000001e6448 t _rb_str_new
00000000001e677c t _rb_str_new_cstr
00000000001e7b10 t _rb_str_new_frozen
00000000001e7a9c t _rb_str_new_shared
00000000001e6914 t _rb_str_new_static
00000000001e80cc t _rb_str_new_with_class
ianks commented 1 year ago

And for good measure, let's specify the features of rb-sys:

diff --git i/Cargo.toml w/Cargo.toml
index 46d4e99..857089b 100644
--- i/Cargo.toml
+++ w/Cargo.toml
@@ -5,8 +5,11 @@ edition = "2021"

 [dependencies]
 gdnative = "0.11"
-rb-sys = "0.9.56"
+rb-sys = { version = "0.9.65", features = ["link-ruby", "ruby-static"] }
 magnus = { git = "https://github.com/matsadler/magnus.git", rev = "52eeaad", features = ["embed", "ruby-static"] }

+[build-dependencies]
+rb-sys-env = "0.1.2"
+
 [lib]
 crate-type = ["cdylib"]
ianks commented 1 year ago

Played around with it and got the basic demo working.

I know nothing about godot, but noticed things broke when embed::setup was run from init rather that _ready. I assume there may be different threads executing the code from those functions, which will break Ruby. To get around this, I do the initialization in _ready.

Another solution for this problem would be to spawn a tokio runtime with a single thread, and ensure all calls to ruby happen inside of a Runtime::block_on call. This will ensure all calls to libruby happen in a single thread.

elct9620 commented 1 year ago

Thanks for your demo. I put the embed::setup in the init because I expect the Ruby should be initialized when Godot loaded it only once.

The _ready in impl Ruby may call many times because it will convert a component (script) and attach to another object in the engine and create them each time when an object is created.

The newer Godot is changing the extension system design, I want to identify the issues caused by magnus or godot-rust's first (e.g. godot-rust cannot support singleton mode and call the init twice) because the extension interface may be changed in the future.