This reverts commit 7332c6a.
Non-static userdata is a special case and can cause segfault if try to serialize it.
Now it should be safe, plus I added non-static userdata destructor to generate better error messages
in case of accessing destructed userdata.
This is a somewhat involved change with two breaking API changes:
1) Lua::coerce_xxx methods now return Option (this is easier and faster than
dealing with Result)
2) rlua numeric conversions now allow more loss of precision
conversions (e.g. 1.5f32 to 1i32)
The logic for the first breaking change is that mostly the coerce methods are
probably used internally, and they make sense as low-level fallible casts and
are now used as such, and there's no reason to confuse things with a Result with
a large error type and force the user to match on the error which will hopefully
only be FromLuaConversionError anyway.
The logic for the second change is that it matches the behavior of
num_traits::cast, and is more consistent in that *some* loss of precision
conversions were previously allowed (e.g. f64 to f32).
The problem is that now, Lua::coerce_integer and Lua::unpack::<i64> have
different behavior when given, for example, the number 1.5. I still think this
is the best option, though, because the Lua::coerce_xxx methods represent how
Lua works internally and the standard C API cast functions that Lua provides,
and the ToLua / FromLua code represents the most common form of fallible Rust
numeric conversion.
I could revert this change and turn `Lua::eval::<i64>("1.5", None)` back into an
error, but it seems inconsistent to allow f64 -> f32 loss of precision but not
f64 -> i64 loss of precision.
Tried to explain the rationale for safety around callbacks in Lua and Scope a
bit better, because every time I don't look at this for a while I forget my
reasoning. I'm not always so great at using the right terminology, so to
whoever reads this, if I got this wrong please tell me.
Uses the same UserData trait, and should at least in theory support everything
that 'static UserData does, except that any functions added that rely on
AnyUserData are pretty much useless.
Probably pretty slow and I'm not sure how to make it dramatically faster, which
is a shame because generally when you need non'-static userdata you might be
creating it kind of a lot (if it was long-lived, it would probably be 'static).
Haven't added tests yet, will do that next.
Callbacks should not be able to capture their arguments and hold onto them,
because the `&Lua` used in previous calls will not remain valid across calls.
One could imagine an API where the specific `&Lua` is simply stored inside the
`Scope` itself, but this is harder to do, and would (badly) encourage storing
references inside Lua userdata.
Ideally, the only way it should be possible to store Lua handles inside Lua
itself is through usafety or the `rental` crate or other self-borrowing
techniques to make references into 'static types. If at all possible this
roadblock should stay, because reference types inside userdata are almost always
going to lead to a a memory leak, and if you accept the risks you should just
use `RegistryKey` with its manual removal.
The documentation describing it being a logic bug to access "outer" callback
handles when inside an "inner" callback is inaccurate, that was only true when
using an older design for handle values.
Also, there is no reason to have a separate 'callback lifetime, because 'scope
is already invariant and just using 'scope seems equivalent.
Vastly simpler and less magical than using a fixed size magical section of the
active stack, and seems to be no slower. The only real downside is that
it *seems* extremely extremely hacky (and to be fair, it is).
This should protect against being able to trigger a stack assert in Lua. Lua
and associated types shoul be able to assume that LUA_MINSTACK stack slots are
available on any user entry point. In the future, we could turn check_stack
into something that only checked the Lua stack when debug_assertions is true.
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, don't bother asserting if the userdata has no metatable, just behave as
though the userdata has no type. This should be impossible to trigger currently
without the debug library, but it is not really that useful of an assert anyway.
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.
So, despite staring intently at the params structure magic in protect_lua_call,
there is still a nasty bug. In the event of an error, the return value of the
parameters structure could be dropped despite being mem::unintialized. Of
course, the actual return values are incidentally always Copy I think, so this
wasn't an actual bug, but I've proven to myself the danger of such dark majyyks.
Just use Option and be done with it, it doesn't have to be so complicated!
Also document why there are a slew of random functions in the ffi module.
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?
And it will work until something fails! Maybe there should be a test that calls
every possible function that invokes to_lua / from_lua with a type where both
directions fail?
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.
When 'debug_assertions' is not enabled, don't bother doing asserts in
stack_guard / stack_err_guard. Also, add an optional feature not enabled by
default to disable LUA_USE_APICHECK in release mode. Once the bugs in rlua that
allow you to trigger LUA_USE_APICHECK are fixed, this feature will be the
default behavior.
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.
It is part of the contract that only LuaRef types constructed from the same
parent Lua state are passed into Lua, so generating a panic there is not an
internal error.
* 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.
CallbackError now, instead of displaying the cause description, instead prints
"callback error: <traceback>". Since the cause is already in the cause chain of
the error, this avoids repeatedly printing the cause of callback errors along
the chain, and also actually prints the callback when using Display on each
error in the chain.
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`.
This cannot be accomplished without using unsafe code, which justifies this addition in my opinion.
Also changes "null" to "nul" to be in sync with `std::ffi` docs. Naming is derived from `CStr::to_bytes_with_nul`, using `as_*` instead of `to_*` since this isn't doing any computation.
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.
Since this is `rlua::Error`, it should be clear that it refers to
Lua-related errors. Downstream crates want to define their own error
enums, which can add a prefix like "lua error:" to disambiguate.
Previously, the traceback would be printed, but not the actual error.
I've removed traceback printing completely, not sure if that's a good
idea. A `Display` impl that outputs multiple lines feels weird.
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'
This was only allowed for `UserData` implementors that are also `Copy`.
This relaxes the requirement to be `Clone` instead.
While `Copy` makes sense to prevent allocations and other potentially
costly operations, other `FromLua` impls already do pretty expensive
stuff, so this seems worth it.
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.
Adds `as_bytes` to view the string as a `[u8]`. Unlike the conversion to
a `&str` slice, this cannot fail.
Adds tests for both functions, which made me notice that `to_str` is
broken when the string contains null bytes, so I made it use the
`as_bytes` method.
Not only was this code not equivalent, it didn't even run since varargs
cannot be used as an upvalue (it's multiple values, after all).
Since Lua does not allow passing 2 sets of variadic arguments to a
function, the resulting code would be *very* complex and would involve
packing both sets of varargs into tables, concatenating them, then
`table.unpack`ing them to finally pass them.
This complex code would only make the docs more difficult to understand,
which is the opposite effect I originally intended with this. Let's just
get rid of this bad equivalence.
> Examples use ?, not try!, not unwrap (C-QUESTION-MARK)
> Like it or not, example code is often copied verbatim by users.
> Unwrapping an error should be a conscious decision that the user
> needs to make.
Rename the following:
LuaNil => Nil
LuaExternalError => ExternalError
LuaExternalResult => ExternalResult
LuaCallback => Callback (internal only)
Use qualified re-exports at the top of the module.
Add a new public 'prelude' module which re-exports everything with a
non-conflicting name (Adds back the Lua prefix), and is meant to be imported
unqualified.
This required a lot of little adjustments where we used std's `String`
before. In downstream code, this shouldn't be necessary, as you can just
do `use rlua::String as LuaString` to disambiguate.
Lua 5.3 has the ability for scripts to define __gc metamethods on
tables, which gives them the ability to "resurrect" userdata after __gc
has been called. This means, __gc can be called multiple times on
userdata. This commit protects against this by simply panicking on
access after resurrection. This is possibly not the best approach?
I know, this is the opposite of PR #17 wishes to do, please don't take this as
an indication that I would wish to do the opposite. I actually want to discuss
PR #17 with you, but I'm not sure about it yet, and my pedantry will not allow
me to let this remain inconsistent in the meantime. This way, either way it's
consistent haha.
It, ahem "should not" be possible to exhaust lua stack space in normal usage,
and causing stack errors to be Err is slightly obnoxious. I have been wanting
to make this change for a while, and removing the callback API from tables makes
this sensible *I think*.
I can think of a couple of ways that this is not technically true, but I think
that they are acceptable, or should be handled differently.
One, you can make arbitrarily sized LuaVariadic values. I think this is maybe a
bug already, because there is an argument limit in Lua which is lower than the
stack limit. I'm not sure what happens there, but if it is a stack based panic,
(or any panic?) it is a bug.
Two, I believe that if you recurse over and over between lua -> rust -> lua ->
rust etc, and call rlua API functions, you might get a stack panic. I think for
trusted lua code, this is morally equivalent to a regular stack overflow in
plain rust, which is already.. well it's not a panic but it's some kind of safe
crash I'm not sure, so I think this is acceptable. For *untrusted* lua code,
this could theoretically be a problem if the API provided a callback that would
call back into lua, then some lua script could force a stack based panic. There
are so many concerns with untrusted lua code, and this library is NOT safe
enough yet for untrusted code (it doesn't even provide an option to limit lua to
the safe API subset yet!), so this is not currently an issue. When the library
provides support for "safe lua", it should come with big warnings anyway, and
being able to force a stack panic is pretty minor in comparison.
I think if there are other ways to cause unbounded stack usage, that it is a
bug, or there can be an error just for that situation, like argument count
limits.
This commit also fixes several stupid bugs with tests, stack checking, and
panics.
The current situation with error_chain is less than ideal, and there are lots of
conflicting interests that are impossible to meet at once. Here is an
unorganized brain dump of the current situation, stay awhile and listen!
This change was triggered ultimately by the desire to make LuaError implement
Clone, and this is currently impossible with error_chain. LuaError must
implement Clone to be a proper lua citizen that can live as userdata within a
lua runtime, because there is no way to limit what the lua runtime can do with a
received error. Currently, this is solved by there being a rule that the error
will "expire" if the error is passed back into rust, and this is very
sub-optimal. In fact, one could easily imagine a scenario where lua is for
example memoizing some function, and if the function has ever errored in the
past the function should continue returning the same error, and this situation
immediately fails with this restriciton in place.
Additionally, there are other more minor problems with error_chain which make
the API less good than it could be, or limit how we can use error_chain. This
change has already solved a small bug in a Chucklefish project, where the
conversion from an external error type (Borrow[Mut]Error) was allowed but not
intended for user code, and was accidentally used. Additionally, pattern
matching on error_chain errors, which should be common when dealing with Lua, is
less convenient than a hand rolled error type.
So, if we decide not to use error_chain, we now have a new set of problems if we
decide interoperability with error_chain is important. The first problem we run
into is that there are two natural bounds for wrapped errors that we would
pick, (Error + Send + Sync), or just Error, and neither of them will
interoperate well with error_chain. (Error + Send + Sync) means we can't wrap
error chain errors into LuaError::ExternalError (they're missing the Sync
bound), and having the bounds be just Error means the opposite, that we can't
hold a LuaError inside an error_chain error.
We could just decide that interoperability with error_chain is the most
important qualification, and pick (Error + Send), but this causes a DIFFERENT
set of problems. The rust ecosystem has the two primary error bounds as Error
or (Error + Send + Sync), and there are Into impls from &str / String to
Box<Error + Send + Sync> for example, but NOT (Error + Send). This means that
we are forced to manually recreate the conversions from &str / String to
LuaError rather than relying on a single Into<Box<Error + Send + Sync>> bound,
but this means that string conversions have a different set of methods than
other error types for external error conversion. I have not been able to figure
out an API that I am happy with that uses the (Error + Send) bound. Box<Error>
is obnoxious because not having errors implement Send causes needless problems
in a multithreaded context, so that leaves (Error + Send + Sync). This is
actually a completely reasonable bound for external errors, and has the nice
String Into impls that we would want, the ONLY problem is that it is a pain to
interoperate with the current version of error_chain.
It would be nice to be able to specify the traits that an error generated by the
error_chain macro would implement, and this is apparently in progress in the
error_chain library. This would solve both the problem with not being able to
implement Clone and the problems with (Error + Send) bounds. I am not convinced
that this library should go back to using error_chain when that functionality is
in stable error_chain though, because of the other minor usability problems with
using error_chain.
In that theoretical situation, the downside of NOT using error_chain is simply
that there would not be automatic stacktraces of LuaError. This is not a huge
problem, because stack traces of lua errors are not extremely useful, and for
external errors it is not too hard to create a different version of the
LuaExternalResult / LuaExternalError traits and do conversion from an
error_chain type into a type that will print the stacktrace on display, or
use downcasting in the error causes.
So in summary, this library is no longer using error_chain, and probably will
not use it again in the future. Currently this means that to interoperate with
error_chain, you should use error_chain 0.8.1, which derives Sync on errors, or
wait for a version that supports user defined trait derives. In the future
when error_chain supports user defined trait derives, users may have to take an
extra step to make wrapped external errors print the stacktrace that they
capture.
This change works, but is not entirely complete. There is no error
documentation yet, and the change brought to a head an ugly module organization
problem. There will be more commits for documentation and reorganization, then
a new stable version of rlua.
Lua should be consistent independent of the way they are produced. pcall in lua
should produce the same sort of error that would be returned by a LuaError
conversion.
The situation is not currently great, because LuaError is not Clone, so passing
a LuaError back into rust will result in the error being "consumed".
There is no single obvious form for a set in lua, and it is not very difficult
to accept a table and convert the sequence values into a set.
Also rename some methods as per discussion.
When talking about "loading" Lua code, it usually means compiling a
chunk of code into a runnable Lua function, but without actually
running it. This makes that clear.
For this to work, both `T` and `E` need to implement `ToLua`. An `Ok(t)`
passes the contained `T`, while an `Err(e)` passes `nil` followed by the
contained `E`. This matches the common Lua idiom used by functions like
`io.open`.
Closes#3
partially for the selfish reason that my submodule setup does not deal well with
ureleased versions of cargo libs where the version number ends with -pre
Allow load to return values, allows reimplementing require() like functions
properly.
Make globals table explicit in Lua, remove Lua::get / Lua::set in favor of
Lua::globals. Allows obeying globals metatable, using other Table functions on
the globals table.
Also added "has" method as shorthand for checking whether a table entry is not
nil.
There should be drastically less ways to cause unprotected lua errors now, as
the LuaTable functions which were trivial to cause unprotected errors are now
protected. Unfortunately, they are protected in a pretty slow, terrible way
right now, but it at least works.
Also, set the atpanic function in lua to call a proper rust panic instead.
I know that Lua::to uses FromLua and Lua::from uses ToLua, but it only
makes sense that Lua::from(42) would return the LuaValue for 42, and
that Lua::to::<i64>(v) would convert the given lua value TO an integer.
The way it was just incredibly backwards.
All lua types should now be at least somewhat usable from rust, if
incompletely so. Any lua value should be readable in Rust as a
LuaValue, pop_value should never panic except in internal logic errors.
Alex Orlenko