Allow to disable drop from running during unwind

[ultimaweapon]

Background

I'm building a Lua binding that using drop to pop the values from Lua stack. I'll start by explaining how to use Lua API to embed it in the application so people who are not familiar with it can understand the problem I faced. You can proceed to next section if you already know about this.

Lua itself is written in C and it provides C API to embed it. The API is stack-based. You pass the value to Lua by pushing it to Lua stack (not stack on the OS thread) and receive the value from Lua by get it from Lua stack. The following is an example to call a Lua function that return a string with foo as the first argument and 5 as the second argument:

lua_pushstring(L, "foo");
lua_pushinteger(L, 5);
lua_call(L, 2, 1); // 2 is the number of pushed arguments and 1 is the number of return values.
const char *ret = luaL_checkstring(L, -1); // -1 is top of the stack.
lua_pop(L, 1); // Remove the returned value, which render ret pointer on the above invalid after this.

If the function does not return a string luaL_checkstring will push an error message then do a long jump to Lua error handler using C longjmp or using throw if it was compiled as C++, which Lua error handler expect this error message on the top of the stack.

How my binding works

My binding requires Lua to compile as C++ so Drop implementation on Rust type can free any resources automatically when Lua trigger an error. I also represent each of the item in Lua stack as a Rust type that has Drop implementation:

pub struct Str<'p, P: Frame>(&'p mut P);

impl<'p, P: Frame> Str<'p, P> {
    /// # Safety
    /// Top of the stack must be a string.
    pub(crate) unsafe fn new(p: &'p mut P) -> Self {
        Self(p)
    }
}

impl<P: Frame> Drop for Str<'_, P> {
    fn drop(&mut self) {
        unsafe { lua_pop(self.0.get(), 1) };
    }
}

P is the parent item in the stack so when drop is running it is guarantee that top of the stack always in the same shape as when the value is constructed. This mechanism works really well since it is effectively zero cost. The only problem is it does not works when Lua trigger any error since top of the stack is the error message instead of the last constructed value. I can use std::uncaught_exceptions() to check inside Drop implementation but it is not an optimal solution. Currently I patched Lua source to put the error message somewhere else before it raising a C++ exception so top of the stack has the same shape when drop is running. It would be better if I can exclude Drop implementation on the above type from running during unwind.

[tgross35]

Can you mark the messages ManualyDrop?

[ultimaweapon]

Thanks for the idea. I don't think that is possible. I just updated my above comment to add more context. Please let me know if you don't understand which part since English is my second language and it quite hard for me to explain complex things.

[ptrca]

don't think that is possible.

Can you elaborate why using ManualyDrop is not possible? Also what the others lua bindings like mlua do in this case? Do you compare your solution and their?

[ultimaweapon]

I know the purpose of ManualyDrop but I don't know how it can solve my problem. The reason I invent my own bindings is because mlua focus on high-level API instead of performance. I did not look at mlua source much but one thing I found is it use luaL_ref on the created table to achieve owned-version of the value. So the goal of my bindings if focus on maximize performance instead of high-level API.

[kennytm]

how about using lua_gettop() to get the absolute stack size before pushing and recover with lua_settop() 🤔

[ultimaweapon]

how about using lua_gettop() to get the absolute stack size before pushing and recover with lua_settop() 🤔

That is one possible solution but it is also introduce an overhead. I prefer my current workaround that patch Lua to put the error out of Lua stack before raising an error than this one.

[tgross35]

If the error is fatal enough that you start unwinding the Rust stack, then it makes sense to assume the lua stack is also going away. So I think your solution does seem reasonable; in the lua exception handler you should copy the error to something allocated and then get that pointer to Rust for printing (something global could work, like errno). You could likely also create a stacktrace here for debugging.

Regarding the original proposal, I think language support is pretty unlikely since there are plenty of workarounds. If you wanted the behavior as-written, you could route lua_pop through an AtomicPtr / thread-local pointer and replace it with a pointer to a nop function when you don't want anything to actually get popped (i.e. as a step in your handler).

[Diggsey]

It's difficult to tell without more code, but I think this approach is inherently unsound. (See Leakpocolypse)

P is the parent item in the stack so when drop is running it is guarantee that top of the stack always in the same shape as when the value is constructed

This is incorrect. There are a variety of ways in which the child item can go out of scope (ie. its lifetime end and the borrow returned) without its destructor being called.

To do this safely, you'd need to use the callback pattern, ie.

stack.with(item, |stack| {
    // Item is pushed onto the stack for the duration of this closure.
});

// Implementation
impl Stack {
    pub fn with<R>(&mut self, item: impl LuaValue, f: impl FnOnce(&mut Self) -> R) -> R {
        lua_push(item.into());
        let result = f(self);
        lua_pop();
        result
    }
}

With this pattern, popping will also automatically be skipped if there's a panic. If you don't want that you could use a local variable with a destructor to automate the popping (this is sound because the lifetime of the local is under your control).

A further problem is that I'm pretty sure allowing an exception thrown by C++ to cross the FFI boundary is UB.

[kennytm]

A further problem is that I'm pretty sure allowing an exception thrown by C++ to cross the FFI boundary is UB.

#2945 (extern "C-unwind") allows exceptions to cross FFI boundary as long as you don't catch it in between.

[Diggsey]

#2945 (extern "C-unwind") allows exceptions to cross FFI boundary as long as you don't catch it in between.

Ah yes, although that doesn't help in this case because @ultimaweapon can't guarantee that users of their crate don't use catch_unwind, so it would still be unsound to allow a C++ exception to unwind through user code.

[ultimaweapon]

This is incorrect. There are a variety of ways in which the child item can go out of scope (ie. its lifetime end and the borrow returned) without its destructor being called.

Thanks for pointing out. Although I occasionally use ManuallyDrop but I totally forgot this case.

[ultimaweapon]

After trying to figure out a solution for this I'm not sure if it is feasible. As @tgross35 stated on the above that it is unlikely for this to happens so I'll close this as the main problem right now is not disable drop from running but how to build a safe API over Lua API. I may end up forking Lua using C2Rust to achieve what I want.

Thanks for everyone.