tammela added inline comments.
================
Comment at: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp:60-78
+static int runCallback(lua_State *L) {
+ LuaCallback *callback = static_cast<LuaCallback *>(lua_touserdata(L, -1));
+ return (*callback)(L);
+}
+
+llvm::Error Lua::Run(LuaCallback &callback) {
+ lua_pushcfunction(m_lua_state, runCallback);
----------------
labath wrote:
> tammela wrote:
> > labath wrote:
> > > I'm confused. Why use lua to call a c callback, when you could just do
> > > `calllback()`?
> > Some Lua API functions throw errors, if there's any it will `abort()` the
> > program if no panic handler or protected call is provided.
> > To guarantee that the callback always runs in a protected environment and
> > it has error handling, we do the above.
> > Delegating this to the callback itself makes it cumbersome to write.
> Aha, I see.
>
> So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but that
> was years ago..), whenever there's a lua exception inside this (c++)
> callback, lua will longjmp(3) back to the lua_pcall call on line 68, skipping
> any destructors that should normally be invoked. Is that so?
>
> If that's the case, then I think this is a dangerous api, that should at the
> very least get a big fat warning, but that ideally shouldn't exist at all.
>
> What's the part that makes delegating this to the callback "cumersome to
> write"? And why can't that be overcome by a suitable utility function which
> wraps `lua_pcall` or whatever else is needed?
>
> The approach that we've chosen in python is to have very little code
> interacting with the python C API directly. Instead, code generally works
> with our C++ wrappers defined in `PythonDataObject.h`. These functions try to
> hide the python exception magic as much as possible, and present a c++-y
> version of the interface.
>
> Now, since lua is stack-based, something like LuaDataObjects.h would probably
> not work. However, that doesn't meant we should give up on the c++ wrapper
> idea altogether. During the intitial reviews, my intention was for the `Lua`
> class to serve this purpose. I still think this can be achieved if we make
> the callback functions take `Lua&` instead of `lua_State*` as an argument,
> and then ensure the class contains whatever is needed to make the callbacks
> not cumerbsome to write.
> So, if I remember my lua correctly (I wrote a c++ lua wrapper once, but that
> was years ago..), whenever there's a lua exception inside this (c++)
> callback, lua will longjmp(3) back to the lua_pcall call on line 68, skipping
> any destructors that should normally be invoked. Is that so?
>
> If that's the case, then I think this is a dangerous api, that should at the
> very least get a big fat warning, but that ideally shouldn't exist at all.
You are right. This escaped me completely.
Lua can be compiled to use `try/catch` instead of `longjmp`, but that is an
exception (no pun intended). Since we rely on the host's library, we need to
assume that it's using `longjmp`.
> What's the part that makes delegating this to the callback "cumersome to
> write"? And why can't that be overcome by a suitable utility function which
> wraps lua_pcall or whatever else is needed?
>
> The approach that we've chosen in python is to have very little code
> interacting with the python C API directly. Instead, code generally works
> with our C++ wrappers defined in PythonDataObject.h. These functions try to
> hide the python exception magic as much as possible, and present a c++-y
> version of the interface.
>
> Now, since lua is stack-based, something like LuaDataObjects.h would probably
> not work. However, that doesn't meant we should give up on the c++ wrapper
> idea altogether. During the intitial reviews, my intention was for the Lua
> class to serve this purpose. I still think this can be achieved if we make
> the callback functions take Lua& instead of lua_State* as an argument, and
> then ensure the class contains whatever is needed to make the callbacks not
> cumerbsome to write.
Any C++ code that runs in a `lua_pcall` seems to face this issue. I checked a
very complete C++ Lua wrapper called `sol2` and they seem to have the same
issue. I don't think there's a way out of it except code discipline.
Lua provides `lua_atpanic` as a way to recover from unprotected throws. Perhaps
we can leverage this to throw an exception, guaranteeing stack unwinding and
avoiding a call to `abort()`. We would then let the callback run unprotected
and delegate any calls to `lua_pcall` to the callback.
Changes proposed:
- Register the panic function on `Lua` ctor
- Let the `Callback` run unprotected
- Wrap the `Callback` call in a `try catch` block
- Change the `Callback` signature to receive `Lua&` and return `llvm::Error`
It looks like it ticks all goals.
- Unprotected Lua errors do not cause lldb to abort.
- Stack unwinding is guaranteed for C++ code interacting with the Lua state.
- Errors are propagated
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91508/new/
https://reviews.llvm.org/D91508
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
