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

Accessing Scope from within a scoped callback #158

Open
ElusiveMori opened this issue Dec 17, 2019 · 5 comments
Open

Accessing Scope from within a scoped callback #158

ElusiveMori opened this issue Dec 17, 2019 · 5 comments
Labels
Future work An issue which would be nice to do, but not one of the current priorities.

Comments

@ElusiveMori
Copy link

ElusiveMori commented Dec 17, 2019

I'm trying to have some userdata with Rc in it. Obviously, the userdata type becomes non-Send, and so cannot be passed to rlua without going through a Scope.

However, I need this data to be returned from within a callback. My flow at the moment is set up sorta like this:

    lua.context(|ctx| {
        ctx.scope(|scp| {
            // all application code basically goes here

            // some_luafn is supposed to be called from within the scope,
            // and returns a callback that yields non-Send userdata
            // this doesn't work at the moment
            fn some_luafn<'lua, 'scope>(scope: &'scope LuaScope<'lua, 'scope>) -> LuaFunction<'lua> {
                scope.create_function_mut(move |ctx: LuaContext, _: ()| {
                    let result = some_method_returns_userdata(data, ext).unwrap();
                    let userdata: AnyUserData<'lua>  = scope.create_static_userdata(result).unwrap();

                    Ok(userdata)
                }).unwrap()
            }


        })
    });

I can't for the life of me figure out the proper lifetimes of some_luafn. I'm not even sure if this use-case is supported.

So, in this issue, I'd like to ask the following two questions:

  1. Is this a supported use-case, and if so, how does it work?
  2. If not, can it be supported, for example by providing a reference to Scope within callbacks created from Scope just like there is a reference to Context already? I believe there should be no soundness issues arising from this, as the callbacks created by Scope don't live longer than Scope itself anyway.

On a slightly related tangent:
Would it be reasonable to provide a version of rlua without the Send requirements all over the place? It seems like a rather niche feature to be able to Send the Lua state, yet it enforces a whole myriad of inconveniences all over the place, despite the fact that Lua itself is single-threaded. At the very least, it feels like the Scope API is somewhat limited at the moment.

I could potentially look into making a PR/fork without the Send requirements if there are no other caveats to this other than wanting to make the Lua struct Sendable.

Thanks for reading this!

@kyren
Copy link
Contributor

kyren commented Dec 17, 2019

Okay I can't answer that well right now because I'm not at a computer, but I'll give it my best shot.

Is this a supported use-case, and if so, how does it work?

It's not one I thought of when I made the scope system, but it might actually work.

I can't test it right now but I'm pretty sure your problem is actually the &Scope reference itself, it's not 'scope, it's for<'a>. I'm not sure it could be made 'scope but it might be sound, I'd have to think about it.

What you could do instead though is create a UserData in some default state (like for example holding a None), move it into the callback, then return the populated version from the callback. It's a bit annoying but it might work?

If not, can it be supported, for example by providing a reference to Scope within callbacks created from Scope just like there is a reference to Context already? I believe there should be no soundness issues arising from this, as the callbacks created by Scope don't live longer than Scope itself anyway.

I'm not sure, but it would require duplicating a lot of interfaces I think, I'd like to try and solve it in a different way first.

Would it be reasonable to provide a version of rlua without the Send requirements all over the place? It seems like a rather niche feature to be able to Send the Lua state, yet it enforces a whole myriad of inconveniences all over the place, despite the fact that Lua itself is single-threaded.

I really want rlua to support both Send and !Send, but it might have to be a compile time flag or something, I don't know how to make it generic. It's important for some use cases that Lua be Send, for example independent scripted systems in an ECS which is backed by a thread pool. Requiring Send allows you to move the whole Lua state if there are no extant references anywhere. But, it would be useful to turn it off.

At the very least, it feels like the Scope API is somewhat limited at the moment.

It is, but it's because it's very tricky to keep sound. IMO it's not actually that limiting you just sometimes can't do something in the theoretical best way (like having to shove something in an Option). It was the thing I could envision that allowed some use cases that were not possible otherwise, or were only possible with rental. It's primary use case actually is to deal with non-'static types, but it sounds like you're using it just to get around the Send limitation, so it makes sense that it would feel heavyweight for that. Maybe there's a simpler way to make !Send types only safe?

The easiest solution is if changing that scope callback to take &'scope was sound, I but in the current design I don't think it is (I'm pretty sure it can be made to be though? Edit: oh wait no it can't, sorry it's taking me a bit to even remember how I designed this)

@kyren
Copy link
Contributor

kyren commented Dec 17, 2019

https://github.com/kyren/rlua/blob/af4d87d1f6175e8d6022ca348f718d7f6ed10cf3/src/scope.rs#L55

That whole comment block describes the situation pretty well, I might be able to get away with yet another lifetime or something but it already hurts my mind as it is.

@kyren
Copy link
Contributor

kyren commented Dec 17, 2019

I still don't have great confidence in the soundness of Context in general, I just couldn't find any more unsoundness in it. I think it would be massively simpler to understand once GATs land and we can change the type of Lua callbacks to the correct type universally quantified over the 'lua lifetime. It might be a good idea to wait until then.

@ElusiveMori
Copy link
Author

Huge thanks for the detailed write-up. I anticipated that the situation was far more complicated than it looked at first glance. I just wish I had the same deep level of understanding of Rust and lifetimes as you :)

What I ended up doing for now is simply forking rlua and removing the Send trait bound throughout the code and un-implementing Send on Lua. A brute-force solution but one of least resistance in my case.

I think a feature flag would go a long way in making rlua far easier to use in single-threaded scenarios. Not sure how that would be implemented either, though. Trait aliases would be useful but they are unstable.

If I come up with anything, I'll send a PR. For now, though, the solution of yanking Send out of the repo works well enough for me.

@kyren
Copy link
Contributor

kyren commented Dec 17, 2019

Huge thanks for the detailed write-up. I anticipated that the situation was far more complicated than it looked at first glance. I just wish I had the same deep level of understanding of Rust and lifetimes as you :)

Thanks, but I probably wouldn't copy how the rlua scope system works in anything else, it's kind of a hack. I'm waiting on GATs to really resolve it, which is why I'd be uncomfortable with an rlua 1.0 before then. All of the lifetimes in rlua are "wrong" right now, so I also wouldn't use that as evidence that I understand anything that well 😛

@jugglerchris jugglerchris added the Future work An issue which would be nice to do, but not one of the current priorities. label Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Future work An issue which would be nice to do, but not one of the current priorities.
Projects
None yet
Development

No branches or pull requests

3 participants