Since we now optionally use stack spaces for handle values, we have to be
mindful of whether our stack handle points to the stack in an outer level of
Lua "stack protection". We now keep track of the "recursion level" of Lua
instances, and do not allow ref manipulation on "outer" Lua instances until the
inner callback has returned. Also, update the documentation to reflect the
additional panic behavior.
Also makes `Lua` and associated types !UnwindSafe and !RefUnwindSafe, which they
should be because they are intensely internally mutable. Lua IS still panic
safe, but that doesn't mean it should be marked as UnwindSafe (as I understand
it).
Okay, so this is kind of a mega-commit of a lot of performance related changes
to rlua, some of which are pretty complicated.
There are some small improvements here and there, but most of the benefits of
this change are from a few big changes. The simplest big change is that there
is now `protect_lua` as well as `protect_lua_call`, which allows skipping a
lightuserdata parameter and some stack manipulation in some cases. Second
simplest is the change to use Vec instead of VecDeque for MultiValue, and to
have MultiValue be used as a sort of "backwards-only" Vec so that ToLuaMulti /
FromLuaMulti still work correctly.
The most complex change, though, is a change to the way LuaRef works, so that
LuaRef can optionally point into the Lua stack instead of only registry values.
At state creation a set number of stack slots is reserved for the first N LuaRef
types (currently 16), and space for these are also allocated separately
allocated at callback time. There is a huge breaking change here, which is that
now any LuaRef types MUST only be used with the Lua on which they were created,
and CANNOT be used with any other Lua callback instance. This mostly will
affect people using LuaRef types from inside a scope callback, but hopefully in
those cases `Function::bind` will be a suitable replacement. On the plus side,
the rules for LuaRef types are easier to state now.
There is probably more easy-ish perf on the table here, but here's the
preliminary results, based on my very limited benchmarks:
create table time: [314.13 ns 315.71 ns 317.44 ns]
change: [-36.154% -35.670% -35.205%] (p = 0.00 < 0.05)
create array 10 time: [2.9731 us 2.9816 us 2.9901 us]
change: [-16.996% -16.600% -16.196%] (p = 0.00 < 0.05)
Performance has improved.
create string table 10 time: [5.6904 us 5.7164 us 5.7411 us]
change: [-53.536% -53.309% -53.079%] (p = 0.00 < 0.05)
Performance has improved.
call add function 3 10 time: [5.1134 us 5.1222 us 5.1320 us]
change: [-4.1095% -3.6910% -3.1781%] (p = 0.00 < 0.05)
Performance has improved.
call callback add 2 10 time: [5.4408 us 5.4480 us 5.4560 us]
change: [-6.4203% -5.7780% -5.0013%] (p = 0.00 < 0.05)
Performance has improved.
call callback append 10 time: [9.8243 us 9.8410 us 9.8586 us]
change: [-26.937% -26.702% -26.469%] (p = 0.00 < 0.05)
Performance has improved.
create registry 10 time: [3.7005 us 3.7089 us 3.7174 us]
change: [-8.4965% -8.1042% -7.6926%] (p = 0.00 < 0.05)
Performance has improved.
I think that a lot of these benchmarks are too "easy", and most API usage is
going to be more like the 'create string table 10' benchmark, where there are a
lot of handles and tables and strings, so I think that 25%-50% improvement is a
good guess for most use cases.
This will potentially panic on Drop of a `Lua` instance, which may be an abort
if this is a double panic, but that is more desirable than such a bug being
hidden.
Previously, on an internal panic, the Lua stack would be reset before panicking
in an attempt to make sure that such panics would not cause stack leaks or leave
the stack in an unknown state. Now, such panic handling is done in stack_guard
and stack_err_guard instead, and this is for a few reasons:
1) The previous approach did NOT handle user triggered panics that were outside
of `rlua`, such as a panic in a ToLua / FromLua implementation. This is
especially bad since most other panics would be indicative of an internal bug
anyway, so the utility of keeping `rlua` types usable after such panics was
questionable. It is much more sensible to ensure that `rlua` types are
usable after *user generated* panics.
2) Every entry point into `rlua` should be guarded by a stack_guard or
stack_err_guard anyway, so this should restore the Lua stack on exiting back
to user code in all cases.
3) The method of stack restoration no longer *clears* the stack, only resets it
to what it previously was. This allows us, potentially, to keep values at
the beginning of the Lua stack long term and know that panics will not
clobber them. There may be a way of dramatically speeding up ref types by
using a small static area at the beginning of the stack instead of only the
registry, so this may be important.
This simplifies the Scope lifetimes, and should make it a compile error for
scope created handles to exit the scope. This should be strictly better, as you
would never WANT to do this, but I hope that I have not caused a subtle lifetime
problem that would prevent passing those created handles back into Lua. I've
tested every situation I can think of, and it doesn't appear to be an issue, but
I admit that I don't fully understand everything involved and I could be missing
something.
The reason that I needed to do this is that if you can let a scope handle escape
the scope, you have a LuaRef with an unused registry id, and that can lead to
UB. Since not letting the scope references escape is a strict improvement
ANYWAY (if I haven't caused a lifetime issue), this is the easiest fix.
This is technically a breaking change but I think in most cases if you notice it
you would be invoking UB, or you had a function that accepted a Scope or
something. I don't know if it's worth a version bump?
If I happen to change the definition of the Callback type alias, instead of
creating a potentially arbitrary transmute, it will now instead fail to compile.
I don't think that the lifetime of the &Lua in the callback and the lifetime of
the &Lua from creating the callback need to be related at all. I'm not sure if
this has any actual effect, but it makes more sense (I think?).
Avoids messy lifetime issues when interacting with other handle types with scope
produced values.
The whole lifetime situation with 'lua on most methods could actually probably
use some looking at, I'm sure it probably has lots of less than optimal
decisions in it.
This also adds a proper comment to the 'scope lifetime to explain that the key
is that 'scope needs to be invariant to make things safe. Disregard my previous
commit message, the real problem is that I had a poor understanding of lifetime
variance / invaraince.
Okay, so this is the fix for the previously mentioned lifetime problem. I
mimicked the API for `crossbeam::scope` extremely closely for `Lua::scope`, and
for some reason things that would not compile with `crossbeam::scope` WOULD
compile with `Lua::scope`, and I could not figure it out.
So I took the crossbeam source and made tiny edits until I determined the
crossover point where invalid borrows would compile, and it was.. not what I
expected it to be. Simply replacing a RefCell<Option<DtorChain<'a>>> with a
PhantomData<&'a ()> would suddenly cause this to compile with crossbeam:
```
struct Test {
field: i32,
}
crossbeam::scope(|scope| {
let mut t = Test {
field: 0,
};
scope.spawn(|| t.field = 42);
drop(t);
// ...anything
})
```
which is precisely the same problem as `rlua`.
To say I am unsatisfied by this fix is a drastic understatement. SURELY this
must be a compiler bug?
The following code should not compile:
```
struct Test {
field: i32,
}
let lua = Lua::new();
lua.scope(|scope| {
let mut test = Test { field: 0 };
let f = scope
.create_function(|_, ()| {
test.field = 42;
Ok(())
})
.unwrap();
lua.globals().set("bad!", f).unwrap();
});
```
yet it does with this commit. However, I have a fix for this, which I do not in
any way understand.
* Make Lua Send
* Add Send bounds to (nearly) all instances where userdata and functions are
passed to Lua
* Add a "scope" method which takes a callback that accepts a `Scope`, and give
`Scope` the ability to create functions and userdata that are !Send, *and also
functions that are not even 'static!*.
Now, simply remove the userdata table immediately before dropping the userdata.
This does two things, it prevents __gc from double dropping the userdata, and
after the first call to __gc, it prevents the userdata from being identified as
any particular userdata type, so it cannot be misused after being finalized.
This change thus removes the userdata invalidation error, and simplifies a lot
of userdata handling code.
It also fixes a panic bug. Because there is no predictable order for
finalizers, it is possible to run a userdata finalizer that does not resurrect
itself before a lua table finalizer that accesses that userdata, and this means
that there were several asserts that were possible to trigger in normal Lua code
in util.rs related to `WrappedError`.
Now, finalized userdata is simply a userdata with no methods, so any use of
finalized userdata becomes a normal script runtime error (though, with a
potentially confusing error message). As a future improvement, we could set
a metatable on finalized userdata that provides a better error message.
Also, during the implementation of this, I noticed a problem with the 0.10
memory safety, which is that luaL_ref is also memory unsafe. I attempted to
change the API to support luaL_ref potentially returning Result, but this change
will cause an enormous amount of API chaos, (just as an example, it becomes
impossible to implement Clone for LuaRef as is). Instead, luaL_ref now is
guarded by gc_guard.
Also make sure that panic messages clearly state that they are internal errors,
so people report them as a bug. Since the only panics left are all internal
errors, just move the internal error message into the panic / assert macros.
- Update readme, changelog, cargo version number in preparation for release
- Remove panicking behavior on recursive callback calls, add additional error
variant for recursive callback errors.
- auto formatting
- add gc control to ffi
- add gc_guard to util functions
- use gc_guard to make util error handling functions never trigger __gc
metamethod Lua errors even without __gc metatable wrapper
- sort of a technicality, don't call luaL_requiref outside of the Lua
constructor, as it could trigger the garbage collector when user code has had
a chance to set __gc metamethods. Changes the API to load the debug table.
First, make sure that `add_methods` cannot trigger another userdata registry
insert, causing an unintended panic. Second, remove `RefCell` surrounding
userdata hashmap, as this change makes it no longer needed. Third, add a
`RefCell` around `Callback` because FnMut means that callbacks cannot recurse
into themselves, and panic appropriately when this happens. This should
eventually be turned into an error.
The stock Lua interpreter doesn't do this either. AFAIK the "base" library is supposed to be loaded into "_G" only as it contains functions like `assert` and `error`.
setmetatable now wraps a __gc method in a cclosure that aborts on error, also
'debug' library is no longer provided. We could provide just the subset of the
debug library that is sound, though.
Also rename to/from/pack/unpack to pack/unpack/pack_multi/unpack_multi, I don't
know if this makes their usage clearer, and it IS a bit confusing that I'm
changing the meaning of the words 'pack' and 'unpack'
I didn't yet document *everything* there is to say (in particular, how
exactly custom Rust errors can be passed through Lua), but I've some
changes to this type in mind that I'll do next.
In resume_with_traceback, always use the coroutine stack for error handling so
we don't miss panics, in both _with_traceback functions remove the temporary
traceback entry from the stack.