mlua-rs/rlua

Returning a closure from a `Scope::create_function`-created function

Closed this issue · 9 comments

In the process of writing rlua-async, I'm trying to make an async fn out of Scope::create_function, and this requires me to actually return a callback from a scope.create_function. Is there a way of doing it? I've tested the following, and they all fail:

Basic scope-in-scope

This one fails with lifetime errors. It's the most obvious one I would have expected to work, though

scope.create_function(move |_, ()| {
    scope.create_function(move |_, ()| {
        Ok(()))
    }
})

returns

error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
   --> src/lib.rs:555:41
    |
555 |                 let f: Function = scope.create_function(move |_, ()| {
    |                                         ^^^^^^^^^^^^^^^
    |
note: first, the lifetime cannot outlive the anonymous lifetime #2 defined on the body at 553:23...
   --> src/lib.rs:553:23
    |
553 |               lua.scope(|scope| {
    |  _______________________^
554 | |                 let r = rc.clone();
555 | |                 let f: Function = scope.create_function(move |_, ()| {
556 | |                     scope.create_function(move |_, ()| {
...   |
567 | |                 assert_eq!(Rc::strong_count(&rc), 2);
568 | |             });
    | |_____________^
note: ...so that reference does not outlive borrowed content
   --> src/lib.rs:555:35
    |
555 |                 let f: Function = scope.create_function(move |_, ()| {
    |                                   ^^^^^
note: but, the lifetime must be valid for the anonymous lifetime #2 defined on the body at 551:28...
   --> src/lib.rs:551:28
    |
551 |           Lua::new().context(|lua| {
    |  ____________________________^
552 | |             let rc = Rc::new(Cell::new(0));
553 | |             lua.scope(|scope| {
554 | |                 let r = rc.clone();
...   |
581 | |             };
582 | |         });
    | |_________^
note: ...so that the types are compatible
   --> src/lib.rs:553:17
    |
553 |             lua.scope(|scope| {
    |                 ^^^^^
    = note: expected  `rlua::context::Context<'_>`
               found  `rlua::context::Context<'_>`

ctx-in-scope

When I try to randomly change the inner scope with lua (my Context), which gives this code:

scope.create_function(move |_, ()| {
    lua.create_function(move |_, ()| {
        Ok(()))
    }
})

I get essentially the same result:

error[E0495]: cannot infer an appropriate lifetime for autoref due to conflicting requirements
   --> src/lib.rs:555:41
    |
555 |                 let f: Function = scope.create_function(move |_, ()| {
    |                                         ^^^^^^^^^^^^^^^
    |
note: first, the lifetime cannot outlive the anonymous lifetime #2 defined on the body at 553:23...
   --> src/lib.rs:553:23
    |
553 |               lua.scope(|scope| {
    |  _______________________^
554 | |                 let r = rc.clone();
555 | |                 let f: Function = scope.create_function(move |_, ()| {
556 | |                     lua.create_function(move |_, ()| {
...   |
567 | |                 assert_eq!(Rc::strong_count(&rc), 2);
568 | |             });
    | |_____________^
note: ...so that reference does not outlive borrowed content
   --> src/lib.rs:555:35
    |
555 |                 let f: Function = scope.create_function(move |_, ()| {
    |                                   ^^^^^
note: but, the lifetime must be valid for the anonymous lifetime #2 defined on the body at 551:28...
   --> src/lib.rs:551:28
    |
551 |           Lua::new().context(|lua| {
    |  ____________________________^
552 | |             let rc = Rc::new(Cell::new(0));
553 | |             lua.scope(|scope| {
554 | |                 let r = rc.clone();
...   |
581 | |             };
582 | |         });
    | |_________^
note: ...so that the types are compatible
   --> src/lib.rs:556:25
    |
556 |                     lua.create_function(move |_, ()| {
    |                         ^^^^^^^^^^^^^^^
    = note: expected  `rlua::context::Context<'_>`
               found  `rlua::context::Context<'_>`

scope-in-lua

At this point I'm randomly trying all the combinations, hoping for one to compile so that I could then think later about my original problem. So I try the last idea that uses a Scope:

lua.create_function(move |_, ()| {
    scope.create_function(move |_, ()| {
        Ok(()))
    }
})

And it still fails, though this time due to scope not having the correct Send/Sync properties:

error[E0277]: `std::cell::RefCell<std::vec::Vec<(rlua::types::LuaRef<'_>, fn(rlua::types::LuaRef<'_>) -> std::boxed::Box<(dyn std::any::Any + 'static)>)>>` cannot be shared between threads safely
   --> src/lib.rs:555:39
    |
555 |                 let f: Function = lua.create_function(move |_, ()| {
    |                                       ^^^^^^^^^^^^^^^ `std::cell::RefCell<std::vec::Vec<(rlua::types::LuaRef<'_>, fn(rlua::types::LuaRef<'_>) -> std::boxed::Box<(dyn std::any::Any + 'static)>)>>` cannot be shared between threads safely
    |
    = help: within `rlua::scope::Scope<'_, '_>`, the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<std::vec::Vec<(rlua::types::LuaRef<'_>, fn(rlua::types::LuaRef<'_>) -> std::boxed::Box<(dyn std::any::Any + 'static)>)>>`
    = note: required because it appears within the type `rlua::scope::Scope<'_, '_>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&rlua::scope::Scope<'_, '_>`
    = note: required because it appears within the type `[closure@src/lib.rs:555:55: 559:18 scope:&rlua::scope::Scope<'_, '_>]`

error[E0277]: `*mut rlua::ffi::lua_State` cannot be shared between threads safely
   --> src/lib.rs:555:39
    |
555 |                 let f: Function = lua.create_function(move |_, ()| {
    |                                       ^^^^^^^^^^^^^^^ `*mut rlua::ffi::lua_State` cannot be shared between threads safely
    |
    = help: within `rlua::scope::Scope<'_, '_>`, the trait `std::marker::Sync` is not implemented for `*mut rlua::ffi::lua_State`
    = note: required because it appears within the type `rlua::context::Context<'_>`
    = note: required because it appears within the type `rlua::scope::Scope<'_, '_>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&rlua::scope::Scope<'_, '_>`
    = note: required because it appears within the type `[closure@src/lib.rs:555:55: 559:18 scope:&rlua::scope::Scope<'_, '_>]`

error[E0277]: `std::cell::Cell<&()>` cannot be shared between threads safely
   --> src/lib.rs:555:39
    |
555 |                 let f: Function = lua.create_function(move |_, ()| {
    |                                       ^^^^^^^^^^^^^^^ `std::cell::Cell<&()>` cannot be shared between threads safely
    |
    = help: within `rlua::scope::Scope<'_, '_>`, the trait `std::marker::Sync` is not implemented for `std::cell::Cell<&()>`
    = note: required because it appears within the type `std::marker::PhantomData<std::cell::Cell<&()>>`
    = note: required because it appears within the type `rlua::scope::Scope<'_, '_>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&rlua::scope::Scope<'_, '_>`
    = note: required because it appears within the type `[closure@src/lib.rs:555:55: 559:18 scope:&rlua::scope::Scope<'_, '_>]`

error[E0277]: `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>` cannot be shared between threads safely
   --> src/lib.rs:555:39
    |
555 |                 let f: Function = lua.create_function(move |_, ()| {
    |                                       ^^^^^^^^^^^^^^^ `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>` cannot be shared between threads safely
    |
    = help: within `rlua::scope::Scope<'_, '_>`, the trait `std::marker::Sync` is not implemented for `*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>`
    = note: required because it appears within the type `std::marker::PhantomData<*mut std::marker::PhantomData<std::cell::UnsafeCell<()>>>`
    = note: required because it appears within the type `rlua::context::Context<'_>`
    = note: required because it appears within the type `rlua::scope::Scope<'_, '_>`
    = note: required because of the requirements on the impl of `std::marker::Send` for `&rlua::scope::Scope<'_, '_>`
    = note: required because it appears within the type `[closure@src/lib.rs:555:55: 559:18 scope:&rlua::scope::Scope<'_, '_>]`

Any idea?

Do you have any idea about how to solve this issue?

I have managed to get the following code to compile, and hoped the 'scope: 'lua and 'callback: 'scope bounds to be valid (based on my reading of the comment in scope.rs), but it looks like these bounds prevent actual usage of the function -- maybe the problem is elsewhere, though, I'm far from being sure with so many lifetimes:

/// Extension trait for [`rlua::Scope`]
pub trait ScopeExt<'lua, 'scope> {
    /// Wraps a Rust function or closure, creating a callable Lua function handle to it. See also
    /// [`rlua::Scope::create_function`] and [`ContextExt::create_async_function`].
    ///
    /// The `where` clause stipulates two additional constraints compared to
    /// [`rlua::Scope::create_function`]. This is required for compiling the implementation, and
    /// should not be an issue, as the lifetimes should always follow them.
    fn create_async_function<'callback, Arg, Ret, RetFut, F>(
        &'callback self,
        ctx: Context<'callback>,
        func: F,
    ) -> Result<Function<'lua>>
    where
        'scope: 'lua,
        'callback: 'scope,
        Arg: FromLuaMulti<'callback>,
        Ret: ToLuaMulti<'callback>,
        RetFut: 'scope + Send + Future<Output = Result<Ret>>,
        F: 'scope + Fn(Context<'callback>, Arg) -> RetFut;
}

// TODO: figure out a way to share code with poller_fn above?
fn scoped_poller_fn<'lua, 'scope, 'callback, Ret, RetFut>(
    scope: &'callback Scope<'lua, 'scope>,
    mut fut: Pin<Box<RetFut>>,
) -> Result<Function<'lua>>
where
    Ret: ToLuaMulti<'callback>,
    RetFut: 'scope + Send + Future<Output = Result<Ret>>,
{
    scope.create_function_mut(
        move |ctx, _: MultiValue<'callback>| -> Result<MultiValue<'callback>> {
            FUTURE_CTX.with(|fut_ctx| {
                let fut_ctx_ref = unsafe { &mut *(*fut_ctx as *mut task::Context) };
                match Future::poll(fut.as_mut(), fut_ctx_ref) {
                    Poll::Pending => ToLuaMulti::to_lua_multi((rlua::Value::Nil, false), ctx),
                    Poll::Ready(v) => {
                        let v = ToLuaMulti::to_lua_multi(v?, ctx)?.into_vec();
                        ToLuaMulti::to_lua_multi((v, true), ctx)
                    }
                }
            })
        },
    )
}

impl<'lua, 'scope> ScopeExt<'lua, 'scope> for Scope<'lua, 'scope> {
    fn create_async_function<'callback, Arg, Ret, RetFut, F>(
        &'callback self,
        ctx: Context<'callback>,
        func: F,
    ) -> Result<Function<'lua>>
    where
        'scope: 'lua,
        'callback: 'scope,
        Arg: FromLuaMulti<'callback>,
        Ret: ToLuaMulti<'callback>,
        RetFut: 'scope + Send + Future<Output = Result<Ret>>,
        F: 'scope + Fn(Context<'callback>, Arg) -> RetFut,
    {
        let wrapped_fun = self.create_function(move |ctx, arg| {
            let fut = Box::pin(func(ctx, arg)); // TODO: maybe we can avoid this pin?
            scoped_poller_fn(self, fut)
        })?;

        ctx.load(MAKE_POLLER)
            .set_name(b"coroutine yield helper")?
            .eval::<Function<'lua>>()? // TODO: find some way to cache this eval, maybe?
            .call(wrapped_fun)
    }
}

You can also find the full repository at https://github.com/Ekleog/rlua-async/tree/broken-scopes (branch broken-scopes), which can allow you to just cargo test if you want to fiddle with it by yourself :)

kyren commented

I think you're trying to use Scope possibly for a purpose it was not designed for, I'm not totally sure.

There's a lot going on here that I don't really have time to get into and understand right now, I'm sorry.

However, if you just want to return a callback from a call to Context::scope, then you can do it like this:

let f = lua.scope(|scope| {
    scope.create_function(|lua, ()| {
        lua.create_function(|_, ()| {
            Ok(true)
        })
    }).unwrap()
});
assert!(f.call::<_, Function>(()).unwrap().call::<_, bool>(()).unwrap());

I imagine you might be trying to do something more complex than that though? You can't hold the scope object inside a scoped callback, and you won't be able to make the lifetimes work out because it would be unsafe. The things captured by the scoped callbacks have to strictly outlive the scope, and if they contain a reference to the scope then also the scope must strictly outlive them, and those two requirements are incompatible. You can make a new scope inside the callback, but I'm not sure what you're trying to do and I imagine that wouldn't help.

It's a bit too much for me right now to understand specifically what you're trying to do or what that project is trying to do, I might be able to come back to it in a few days and take a closer look.

Hmm… right, the ctx-in-scope example I gave was stupid for having forgotten to take lua as a closure parameter, sorry for that!

Unfortunately, this solution doesn't solve my issue: the closure I'm passing in the callback isn't Send -- as far as I understand, the closure I'm passing it is !Send + 'scope where 'scope is the scope of outer scope object.

In other words, I'm trying to make a scoped lua callback that, when it is called, will generate another callback with the same scope (and the same captures, not capturing anything additional).

A few code lines are worth a hundred words: what I'm trying is something like this:

let wrapped_fun = [scope|ctx].create_function(move |ctx, arg| {
    let future = Box::pin(non_send_non_static_callback(ctx, arg));
    // future is non-Send non-'static, but has the same lifetime as non_send_non_static_callback
    [scope|ctx].create_function_mut(move |ctx, _| {
        do_things_with(future)
    })
});
return wrap_once_again_in_lua(wrapped_fun);

All this, with the objective of doing something similar to this function, but for Scope::create_function

kyren commented

I'm still not following exactly what's going on, but if you want to return a scoped callback from another scoped callback, you could create the two scoped callbacks in the scope as normal, then give one callback to the other via Function::bind.

Sometimes when something is very hard to do using rlua, you can solve it by using plain Lua instead, even if you just have to write Lua scripts to do it.

If both functions use the same scoped variables, I really don't understand why one function has to return the other unless the second function also needs arguments from the first. But, if it needs arguments from the first, you could still maybe accomplish this using Function::bind to also give the other function the arguments? But again, stuff like this is generally easier to accomplish in plain Lua.

Hmm… At first I was like “YES! What I was missing was Function::bind!”, but… upon more thought, I think it couldn't really work, because the future object of the example in my previous message, that I would need to bind, is a Pin<Box<T>> where T is a type unknown to me (though I know it's T: 'scope + Future<Output = ...>).

So… I think I can't just convert it to a lua value for use in bind?

(Disclaimer: I've actually started using rlua with this rlua-async project, so maybe I'm missing something obvious like Function::bind was… in which case I'm really sorry about that!)

I had another semi-epiphany: I thought I could use UserData to do what I needed to do… but scope.create_nonstatic_userdata has the same issue as scope.create_function: it cannot be called inside scope.create_function.

So I'm getting kind of at a loss about how to do it.

In a few words: I'm trying to return, from a scope.create_function, a Lua value that has a lifetime such that it could be constructed outside the scope.create_function with the same scope, but not with a regular context.

Right now my best bet is the following:

struct FutToPoll<RetFut>(Pin<Box<RetFut>>);

impl<Ret, RetFut> UserData for FutToPoll<RetFut>
where
    Ret: for<'all> ToLuaMulti<'all>,
    RetFut: Future<Output = Result<Ret>>,
{
    // ...
}

// ...
        let make_fut = scope.create_function(move |ctx, arg| {
            let fut: Pin<Box<RetFut>> = Box::pin(func(ctx, arg));
            scope.create_nonstatic_userdata(FutToPoll(fut))
            // or
            Ok(FutToPoll(fut))
        })?;

But none of these can work, the first because I can't take a scope inside a scope, and the second because RetFut has a lifetime of 'scope (ie. compatible with scope's Scope<'lua, 'scope>, but not with regular ToLua)

I just “solved” this issue with transmute to force an extension of the lifetime and addition of the Send bound, so that I could just create the userdata with ToLua.

I assume that, given I know the lua object will be dead before the end of 'scope, it should be OK from a safety perspective, but… is it actually?

The code looks like this: https://github.com/Ekleog/rlua-async/blob/broken-scopes/src/lib.rs#L173-L207

I'd love it if you were to give me an opinion about whether this is actually safe, or whether something I can't think of inside rlua actually breaks it, before I release it :) (also, if you have any other idea about how to do that without that transmute, it'd be even better!)

Also, for what it's worth, here is a small example that should faithfully reproduce my issue without involving all of the Future stuff: managing to adjust foo (without touching its signature, hopefully) to actually compile would probably yield a solution to my issue:

fn foo<'lua, 'scope, RetFn, Ret, Arg, F>(scope: Scope<'lua, 'scope>, generator: F)
where
    Arg: for<'all> FromLuaMulti<'all>,
    Ret: 'scope + for<'all> ToLuaMulti<'all>,
    RetFn: 'scope + for<'all> Fn(Context<'all>) -> Result<Ret>,
    F: 'scope + Fn(Arg) -> RetFn,
{
    scope
        .create_function(move |ctx, arg| {
            let f = generator(arg);
            ctx.create_function(|ctx, ()| f(ctx))
        })
        .unwrap();
}

The type, written in non-Rust syntax and without lifetimes to make it more legible, looks like that:

foo :: (Scope, (Arg -> (Context -> Result<Ret>)))

And foo creates a function that calls its second argument with the given arg, and returns a function that takes in the Context and returns the actually-wanted result.

However weird this may look, it's the way futures work, so there's little changing the problem to fix things up, but doing things like returning a UserData with a method instead of returning just a function would be perfectly fine. (In practice, the created function is immediately passed to a lua function, that calls it with the passed arguments and then repetitively calls the inner function)

Oh. Third epiphany, this time the good one! Using one UserData with create_nonstatic_user_data that mutates itself to handle the arguments actually works for me, without unsafe! The changes can be seen at Ekleog/rlua-async@b90f2d3 , in case you're ever interested.

That said, there is one thing I wonder about: would it not make sense to have Scope::create_function just pass itself as an argument of the closure, in addition to Context and the FromLuaMulti argument? This way, the thing I was initially trying would have trivially worked, and I would think it's not unsafe… is it?

Hi,
I'm glad you've got it working!
I'm not completely sure, but I don't think you gain a lot if the Scope::create_function callback is given the scope, even if it can be made to work. Since any references in the closure still need to be to objects that outlive 'scope, you can't do much that's different from a callback made at the outer level.
I'm closing this as I think it's resolved, and I don't think passing the scope into the create_function callback is viable.