-
Notifications
You must be signed in to change notification settings - Fork 117
rlua is looking for maintainers! #172
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
Comments
(Also copied from the same reddit discussion) For anybody who's interested in working on this, here's a quick wishlist of things that I think
Making this change would immediately remove much of the very delicate and honestly sort of sketchy logic in the callback creation process and the entirety of the "scope" system and make it more obviously sound. However, since I screwed this up when I initially created |
To continue the reddit discussion, it seems like there are a few of us that would be interested in taking on maintainer-ship by committee if that is an acceptable solution. |
I'm a fan of both Rust and Lua, though I don't have the time to commit to |
🗣️ to @Grokmoo of https://github.com/Grokmoo/sulis And if someone knows how to get in touch with @khvzak of mlua that'd be great! (edit: found their email on crates, sent a ping.) |
Thanks @kyren for looking into my work on the My long-term goals are partially the same:
Regarding your comment in the reddit discussion:
However, it's super hard (or even impossible) to guarantee safety in module mode where I'd be happy to get in touch and discuss about unsoundness problem that |
I think this might be the most important point to discuss. I think there is a more restricted form of soundness that is actually what people want that is still very useful to have, and I think the main Rust <-> Lua bindings system, whatever it is, should work very hard to maintain it. Usually when people ask if an API is sound they ofc mean that no UB is possible without writing So, for this library, "soundness" would mean bindings soundness, but as a way to make sure that people understand this, This is (AFAIK currently) the state of the What I was worried about was there being an obviously best Lua bindings crate that does not attempt bindings soundness at all. You could make the case that since Lua is so unsafe, that there's no point anymore, but I definitely don't think that's true. I think that the kinds of things you can do in Lua that are unsafe are somewhat obvious: load C libraries, the debug table, bugs in the interpreter etc, but the kinds of things you can do that are unsafe in the Lua C API are anything but obvious, because the C API is so freakishly hard to safely use. It's possible that your goal with |
There's some interesting perspectives: https://docs.rs/dtolnay/0.0.9/dtolnay/macro._03__soundness_bugs.html
I don't think that's a useful definition of soundness. The link above doesn't define it precisely, but my view is that the most useful definition of soundness is that UB is impossible when all code is either safe or uses One of the examples for making |
Yeah, that's a better definition obviously but in this case that's still not it, becasue there is no contract with You're correct in saying that simply writing unsafe at all does not give someone carte blanche to cause UB, that's not really what I meant to say, I was just skipping a lot of the detail. If the contract of I promise I already understand what soundness is. Obviously an API that has an
I'm just arguing for "bindings soundness" even though the contract must technically be that any method at all can cause UB, I just want to limit how that UB may occur. Also just to be clear I'm not saying @khvzak is suggesting this strawman definition either, I'm just trying to make sure our definitions are aligned. I think that even though someone can pass a |
I think the |
FWIW (I've separately wrapped Lua in a Rust crate but haven't got around to publishing it), my aims for safety/soundness sounds in line with to the above:
My use cases are Lua as configuration - the user isn't malicious but may make mistakes and may be handling malicious data (e.g. from the web or e-mails). I think rlua is doing a better job than my own bindings and is likely to be better maintained and has had more thought put into safety, so I may look into switching over. I'm interested in being involved but do not have time to be a full maintainer. |
Am I correctly understanding, that the following 100% rust safe code using rlua, which 1) modify RO variable 2) causes UB; is sound? fn main() -> LuaResult<()> {
let lua = Lua::new();
lua.context(|ctx| {
let memcpy: LuaFunction = ctx.load(r#"
local ffi = require("ffi")
ffi.cdef[[
void * memcpy(void *, const void *, int);
]]
return ffi.C.memcpy
"#).eval()?;
let a: i32 = 0;
let b: i32 = 255;
// Set "a" to "b"
memcpy.call((&a as &i32 as *const i32 as i64, &b as &i32 as *const i32 as i64, 4))?;
println!("{}", a); // Prints 255
memcpy.call((&a as &i32 as *const i32 as i64, LuaNil, 4))?; // UB; Segfault
Ok(())
})
} ffi is installed from this repo.
Could you please provide an example(s) of unsoundness in rlua 0.15, for better understanding ? The Scope module was removed in 0.3 since my higher priority was releasing async/await support which does not play nice with Scope system. I'm planning to return Scope support in the next release, but without
I definitely not against bindings soundness, if it's possible and does not affect API usability. Soundness is a great advantage!
|
I'm suggesting that
That's not bindings unsoundness, that's the trap I don't want to fall into, not to let perfect be the enemy of good. Since nothing in Lua can be perfectly sound without a complete sandboxing solution which is just not how people use Lua, it's not useful to mark every method that can trigger Lua code to run (which is nearly every method) as unsafe. Just mark
Yeah the scope system was maybe the main one, but also there were other bugfixes, and I re-designed some of the lifetimes at some point after better understanding the mess I had gotten myself into with the dirty lie. If you didn't include the code that removes the need for the Like I said it's possible that mlua already intends to be "bindings sound" and I just misunderstood the README changes. |
I appreciate the shout out @erlend-sh. I will make more of an effort to understand the internals of rlua and see if that takes me to where I could make a useful contribution. I'm also available to help out with more administrative or devops type stuff if the need arises. |
Maybe to mark
After forking rlua 0.15 I spent many time reading the code and changing it. I didn't include Personally I don't use Scope subsystem and doubt a bit about use cases. All the tables/strings/objects/etc created inside executed scoped functions will be alive. Only I also doubt a bit the appropriateness of adding
I hope so :) I believe there is no unsoundness at the moment (with the exception of UB caused by loading C code). I need to review the README. |
It would be great if you could share the details there. We've probably both fixed different bugs since then :(
To safely use non-Send and non-'static data from Lua
Those are the only things that can (directly) store non-Lua data.
because of the bad decision in the lifetimes of the callbacks, ultimately.
And to fix the soundness of the scope system. I think at this point rlua is just hard forked, oh well. |
So I looked into it some more, mlua is unsound in the same ways rlua was unsound pre-0.16, even with the scope system removed. The following segfaults on mlua master (in release):
@khvzak You should really read #97 and understand what I was saying there, it describes the "dirty lie" I keep talking about. The root issue of this, the difficulty with proving the soundness of the scope system and arguably the need to create the context system all come from the fact that I chose the "wrong" lifetimes for callbacks way back when. Fixing this bug either requires making all callbacks into macros OR the context system, and since the context system came with the benefit of not requiring ownership checks on references I went that route. If I could pick one again it would be to have the correct lifetimes on callbacks and to just require macros to create callback functions, or to find SOME other way. I'm sorry I left you with this mess of a problem, I don't really like any part of it. I wish rust had GATs and it could be fixed properly, I think actually if you're going to remove the scope system and context system that the next step is to make that change and just require macro wrappers on callbacks, and then restore some kind of lifetime sanity. It's unfortunate that rust can't express this right now, but I think the correct solution is to just live with it and require macros until such a time where rust can express it. I also wish we could have worked together, I would have discussed alternatives to the context system if I had known it was problematic, though I think every solution I found was problematic in one way or another. I don't blame you for forking rlua, I mean I've done exactly the same thing... it's hard sometimes if you want to make a lot of changes to a project that might be controversial to work to find common solutions, I mean I'm definitely guilty of it myself. I dunno what to do now. mlua has a lot of nice new features and it's clear that you've done a lot of really good work on it. It would be great to leave people with a single, sound bindings system to use rather than the current situation of two bindings systems with different trade-offs and features and levels of safety. |
@khvzak could you say more about your main use case for Lua? I see you're also doing stuff with HAProxy; are you using Lua primarily for web & server stuff? It is quite likely that No userbase fragmentation (i.e. more prospective contributors), active maintenance by @khvzak as long as he has itch to scratch, backed up by @kyren in an advisory capacity drawing from her several years of production experience with Lua, plus the existing rlua community helping to keep things tidy. |
Thanks @kyren ! Seems I made a big mistake and didn't went through the closed issues and history of rlua :( I now understand the depth of the pre-0.16 issues and feeling quite guilty for starting to make changes without a sufficient understanding of the rlua complexity and motivation behind the 0.16 changes.
In the old reddit discussion you're saying that "This is not a practical problem for me anymore, because I have since moved onto using Rc / Weak". Just curious, what's happened with this solution? There is definitely a high chance of memleaks while using Rc.
I definitely like the idea of creating an org-managed library. Divergence between the libraries should not be a big issue I hope. For now I think I need to make step back and take some time to properly think about how to get out of this trap. This kind of problem is quite new to me. I'm going to play with multiple approaches to find a solution. Personally, I tend to move to use macros, as benefit this would allow to pass function arguments as an unpacked list rather than tuple.
You're right, my use cases are quite simple and I use Lua (Rust) primarily for server stuff. Single Lua state, mostly module position. Running code inside haproxy (Lua 5.3) and openresty (LuaJIT) platforms. |
That's okay, I'm glad we're on the same page now!
I don't even remember, it's been too long, but that solution didn't go anywhere.
Yeah I agree here, it seems like the two libraries should merge and be managed by a separate organization.
Yeah I think I agree, any solution that keeps the incorrect lifetimes is fighting the the borrow checker in deeply wrong ways and is really unsustainable. A macro solution is inconvenient but is correct and forwards compatible with a better solution involving GATs, also you're correct that you also don't have to pack all your arguments in tuples anymore.
Just so you're aware of the use case, if you use Lua for game development you need both a Lua object that is Send and also the scope system, that's more or less why they exist. The |
Hey, mate I can help. Feel free to contact me. 🦄 |
Hi @kyren, Also, when talking about supporting Lua 5.4 and luaJIt/5.2: were you thinking of Cargo features to select one at a time or something else? |
I think using cargo features for this is basically fundamentally wrong, since cargo features are not supposed to be mutually incompatible. HOWEVER, I think everyone in similar situations does exactly this anyway, and I'm already guilty of it too with the |
Darn, I was hoping you'd thought of a better way. :-) You'd want something a bit like Cargo features but which are more like an enum than a bool ("lua=lua5.3"). I hadn't really thought of anything better than having an ugly bunch of nested |
There are really "bold" solutions like using environment variables at build time or something. I guess maybe that's not a completely ridiculous idea since other -sys crates do use environment variables to find specific system libraries. This would be the first time I've seen it used to determine actual library features and API though, but I GUESS you could make the argument that it's not wrong? Edit: If you actually think that's a workable idea what I'd think you'd do is use environment variables for all of it, point to a lua library with an environment variable and either auto-detect the lua version or specify a hard-coded lua version also in an environment variable, and which lua version you link to determines specific parts of the API. Also this would allow you to link to any arbitrary API compatible implementation of Lua. |
Oh I forgot to say, this is a wonderful idea and I have no objections. |
Environment variables don't sound good - I'm not sure how that would work with dependencies (especially nested dependencies). What happens if you have two dependencies which both depend on rlua (say because they add new types/functions)? But I admit I haven't looked at how -sys crates use environment variables - I'll look into that before making any real judgements. And I'll raise those issues when I get a few minutes. |
I have created those issues. Let me know (or just fix) if I've misrepresented anything or messed up the copy and paste! |
There hasn't been much activity here recently, but I've started thinking about making some progress (if slowly) in rlua.
I'm definitely not going to have a deadline - no guarantees here, I mainly wanted to establish a direction (if not velocity). |
Well, I've got to this issue in my triage. I don't think there's any reason to keep this ticket open - while more maintainers would be welcome, I think the "looking" for maintainers has happened and I'm it. :-) |
@kyren I was just thinking about doing a minor release (0.17.1 with #211) and realised I probably need your help. First to check if you have a release checklist/process (though I'm sure I can figure something reasonable out), but mainly because presumably it currently needs your key to publish to crates.io. Are you able to help sort something out for that? |
@jugglerchris, I can't help with the release checklist, but Kyren also made me an owner of the rlua pacakge on crates and I can help with getting a release up. |
@azdle Thanks! So when I think things are ready, I can get you to run |
Definitely. You can @ me on the PR (if you do one) or send me an email (in my GH profile).
…On Thu, Sep 2, 2021, at 16:06, Chris Emerson wrote:
@azdle Thanks! So when I think things are ready, I can get you to run
`cargo publish` on the right branch?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#172 (comment)
|
@khvzak have the unsound parts of mlua addressed here been fixed? If not could someone summarize what's unsound about mlua ? |
@makspll it's fixed. I don't have any open issues about mlua unsoundness. mlua moved far way forward since beginning of this discussion. |
Hmm that's fantastic, in this case I would propose that rlua be retired to focus community efforts on a single package. Are there any benefits rlua has over mlua at this point in time ? @jugglerchris |
@makspll I don't know enough about mlua to answer that - I'm afraid I haven't been following it. From scanning https://crates.io/crates/mlua/ it sounds like lots of good stuff has been done (and it's getting more downloads). There are obviously some big design differences, for example no sign of rlua's |
@jugglerchris that's very fair. @khvzak would you mind elaborating on the safety without |
mlua uses bounded lifetimes for functions and a single coroutine-aware Lua instance. Neither mlua nor rlua can guarantee absolute safely and |
@khvzak I am not saying mlua is unsound but rather wanting to spark discourse around the possibility of unifying the community behind one package. If you don't believe it possible or don't think it to be right direction I totally understand. It just seems that there items in the roadmap for both packages (for example wasm support) which overlap, but I feel like development time would be more effectively spent if the workload was reduced and collaborators increased! Do let me know if either of you disagree though! |
@makspll I agree that unifying and expanding a community around one package would be beneficial, but I don't see a way how this can done. |
rlua only joined the amethyst organization because they had experience with managing open source projects and it only happened because rlua lost its original maintainer. So, I don't see a reason for mlua to join the amethyst organization. the context API was (as far as I know) made to solve the problems that @makspll referred to. With mlua having solved those same problems in others ways I don't see why mlua should get the same context based api. Now, the question if one should be deprecated over the other is a totally different question which I don't really have an opinion about. I also doubt that this issue is even the correct place to discuss that. Maybe it is best to move that to something dedicated to that to increase the chance of people seeing it and chime in compared to having that happen in a long closed issue? |
Agreed that this isn't the best place to discuss it. I think probably the answer is that the two projects have slightly different aims and are complementary. I'd like to learn more about how mlua differs, though, in any case. Is there a good starting point to learn about the big differences? |
Copied from a reddit discussion here:
The text was updated successfully, but these errors were encountered: