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

Unable to pass functions as arguments to create_function #73

Closed
Innectic opened this issue Mar 2, 2018 · 4 comments
Closed

Unable to pass functions as arguments to create_function #73

Innectic opened this issue Mar 2, 2018 · 4 comments

Comments

@Innectic
Copy link

Innectic commented Mar 2, 2018

let on_function = lua.create_function(|_, (name, handler): (String, fn(String))| {
	handler(name);
	Ok(())
})?;

I've also attempted with rlua::Function, but that seems to not work either.

Error:

error[E0277]: the trait bound `(std::string::String, fn(std::string::String)): rlua::FromLuaMulti<'_>` is not satisfied
  --> src/modules/api/event_handler.rs:17:25
   |
17 |         let on_function = lua.create_function(|_, (name, handler): (String, fn(String))| {
   |                               ^^^^^^^^^^^^^^^ the trait `rlua::FromLuaMulti<'_>` is not implemented for `(std::string::String, fn(std::string::String))`
   |
   = help: the following implementations were found:
             <(B, A) as rlua::FromLuaMulti<'lua>>

Is there someway to pass a function that I'm missing?

@kyren
Copy link
Contributor

kyren commented Mar 2, 2018

You can use Lua::create_function to turn a Rust function into a Lua one.

What you did would work if there were automatic ToLua conversions for functions, but there aren't right now. Arguably there could be though, so I'll look into it!

Edit: Whoops, I misread this entirely, I was out last night 🍸 and replying on my phone, and replied without thinking. Let me try this again:

Try something like this:

    let lua = Lua::new();
    lua.create_function(|_, (name, handler): (String, Function)| {
        handler.call::<_, ()>(name)
    }).unwrap();

You can't accept a Lua function as a function pointer because it has to be a handle into the Lua state. Lua function handles are just Function, and you have to call them with the Function::call method. I should add an examaple to the guided tour that goes over this.

@Innectic
Copy link
Author

Innectic commented Mar 3, 2018

Ah ha, that seems to work! Thank you so much!

@Innectic Innectic closed this as completed Mar 3, 2018
@Innectic
Copy link
Author

Innectic commented Mar 4, 2018

Hi, I seem to have run into another issue while trying to store a copy of the function:

Code used:

	fn setup_module_globals(&mut self) -> rlua::Result<()> {
		let on_event_function = self.lua.create_function(|_, (name, handler): (String, rlua::Function)| {
			self.events.entry(String::from(name)).or_insert_with(Vec::new).push(handler.clone());
			Ok(())
		})?;
		self.lua.globals().set("on_event", on_event_function.clone())
	}

Error given:

error[E0277]: the trait bound `*mut rlua::ffi::lua_State: std::marker::Sync` is not satisfied in `&mut modules::module::Module<'a>`
  --> src/modules/module.rs:71:36
   |
71 |         let on_event_function = self.lua.create_function(|_, (name, handler): (String, rlua::Function)| {
   |                                          ^^^^^^^^^^^^^^^ `*mut rlua::ffi::lua_State` cannot be shared between threads safely
   |
   = help: within `&mut modules::module::Module<'a>`, the trait `std::marker::Sync` is not implemented for `*mut rlua::ffi::lua_State`
   = note: required because it appears within the type `rlua::Lua`
   = note: required because it appears within the type `modules::module::Module<'a>`
   = note: required because it appears within the type `&mut modules::module::Module<'a>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&&mut modules::module::Module<'a>`
   = note: required because it appears within the type `[closure@src/modules/module.rs:71:52: 78:4 self:&&mut modules::module::Module<'a>]`

It's due to the calling of self.events, which is just a hashmap of Strings to a vector of rlua::Function. I've attempted using a Mutex, to no avail.

@Innectic Innectic reopened this Mar 4, 2018
@kyren
Copy link
Contributor

kyren commented Mar 4, 2018

You're trying to create a Lua closure that captures self by reference, without using Lua::scope or similar. I don't know why specifically you're getting an error about Sync requirements, but that's not the worst error you'll get even if you managed to fix that. It looks like you're also trying to keep a HashMap of Functions on the type that setup_module_globals is a member function of, which you're also going to run into problems with.

I think I see what you're trying to do, and there are a lot of ways to accomplish this. I can't know which one is the best for you, but I can roughly describe some of the solutions.

Ultimately, you need to put some thought into what owns things like self.events. Right now, I can see that you're tying yourself into a bit of a knot, where lua needs to own self to get at self.events, but self owns lua, and also self will have a self borrow for from self.lua to self.events.. you probably need to take a step back and understand some of the limitations here first.

Lua::create_function requires that passed in functions are 'static. This means, that passed in functions cannot have any live references, and that's the first problem you're running into. You want this, because there's no way of knowing when anything passed into Lua will stop being accessible. Since there's no lifetime in Lua to declare that it might hold such a reference, the Rust lifetime system wouldn't be able to help you and you could easily get a use after free crash, so that can't be allowed.

(side note: there's an open issue about optionally adding a lifetime to Lua so that you could always pass in references to things you could prove outlived Lua itself, but that wouldn't actually solve your problem and there are other solutions which basically eliminate the need to do this in the first place.)

You can fix the self reference problem by doing something like putting your events map into an Arc<Mutex<>>, and giving the Lua callback a clone to that Arc. That still won't solve all your problems though, because what your events map seems to hold are rlua::Function, which hold references to Lua and thus.. aren't 'static.

There are two rules about rlua reference types that you're butting up against:

  1. rlua reference types (Table, Function, etc) hold references to Lua, and are not really designed to be stored along-side Lua, because that would require self borrows. You CAN make structs in Rust that self borrow, but you really don't want to go down that road, it's very complex and you 99.999% of the time don't need it.
  2. rlua reference types are also not designed to be stored inside userdata inside Lua. When I say userdata here, I mean both actual UserData types and also things like Rust callbacks. For one, there are some actual safety issues if you were able to do that, but more importantly Lua itself is not really designed for this either. Lua's C API does not provide a way of telling Lua that a userdata contains a registry reference, so you would end up potentially making something that cannot be garbage collected. The actual issue is very complicated but you can read about it some here.

There is, however, a way around these restrictions. rlua provides a reference type that gets rid of all the above problems and is safe, but lets the user deal with any potential garbage collector issues: RegistryKey. You can create these with Lua::create_registry_value, and get them back out with Lua::registry_value, and they're sort of "manually managed references". So, you could use an Arc to share events, and have it contain RegistryKeys, then get the registry keys out when you need to call those events.. but this solution is really complex and prone to garbage collector problems, so there might be a better solution.

One potential solution that prevents you from needing an Arc<Mutex<>> is just to use Lua::scope to access self from a callback, if you can live with the callback being temporary. Another potential solution is to simply keep the map of callbacks inside Lua. You could do this very directly with a global table , but if having the global table be directly accessible by Lua is a problem you can instead use a single RegistryKey for the table or more simply with "named registry values" (see Lua::set_named_registry_value).

Generally when you start running into lifetime problems with rlua you need to take a step back and think about how the ownership is actually going to work. There are some added complexities due to Rust borrow checking of course, but generally the problems you run into are not "just" borrow checking errors, they're indicative of real problems that you would actually run into were the borrow checker not there.

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

No branches or pull requests

2 participants