lupa icon indicating copy to clipboard operation
lupa copied to clipboard

Added option to pass already created LuaState

Open xthebat opened this issue 3 years ago • 4 comments
trafficstars

Hello, I stumble upon limitation of lupa library.

I some cases already created lua_State required to be used. But there is no possibility to pass pointer to lua_State in LuaRuntime. For example Wireshark has embedded Lua interpreter and this interpreter has bindings to Wireshark API. I wanted to write plugin (dissector) using Python because it is much more convenient than Lua for me (a lot libraries, familiar syntax, good IDE, etc). So it is not very hard to start Python interpreter using simple Lua C extensions module but without wrappers like lupa provided, I need to also write these wrappers by myself. So I wrote this simple patch to pass pointer of Lua interpreter into LuaRuntime and it works! I think may be it worth to be added to your library.

Regards

xthebat avatar Sep 22 '22 13:09 xthebat

Mind adding some tests?

Le0Developer avatar Sep 23 '22 16:09 Le0Developer

Thanks. I don't think it's that easy, though.

Lupa assumes that it owns the lua_State, and will deallocate it when the LuaRuntime is closed. It doesn't know that the lua_State is actually externally owned and that its lifetime may not be linked to the lifetime of the LuaRuntime. That's one issue.

Another thing is that LuaRuntime.__init__() does some setup of the Lua state and assumes that all of this is safe to do, because the Lua state was just created. That may be an unsafe assumption depending on where the lua_State originated from.

It's also unclear to me what you are passing into the constructor here. Is it the pointer address of the lua_State as a Python int? What if I pass 0 or 123 from Python?

I'm not generally opposed to such an interface, but then it should be provided as a C-level API and not be made publicly available to Python programs.

An example of your own usage could help to understand how you intend this to work.

scoder avatar Sep 26 '22 07:09 scoder

Sorry for long response, I was bit busy on work.

@scoder

Lupa assumes that it owns the lua_State, and will deallocate it when the LuaRuntime is closed. It doesn't know that the lua_State is actually externally owned and that its lifetime may not be linked to the lifetime of the LuaRuntime. That's one issue. Another thing is that LuaRuntime.init() does some setup of the Lua state and assumes that all of this is safe to do, because the Lua state was just created. That may be an unsafe assumption depending on where the lua_State originated from.

I've added flag self._lua_allocated in 59a72a312c14c155d5af43e890932fec74b54de4 commit that indicate whether lua_State passed from outside or owned by LuaRuntime. In __cinit__ and in __dealloc__ I've added code to check for call lua.luaL_openlibs and lua.lua_close if LuaRuntime own pointer. It works for me, of course it doesn't mean it works everywhere :)

It's also unclear to me what you are passing into the constructor here. Is it the pointer address of the lua_State as a Python int? What if I pass 0 or 123 from Python?

I've reworked it in 32fd682cf621fae7288d840af1be8bd249418546. Now state is Python int that should contain pointer to lua_State. Yeah, If we pass int that contains invalid pointer then program goes to segfault :( ruthless C world. In last commit no check was done to prevent this situation. I think this thought can help to avoid it for common Python user:

it should be provided as a C-level API and not be made publicly available to Python programs.

But all initialization LuaRuntime done in __cinit__ and lua_State* should passed to it for correct initialization also for case external pointer. To hide such initialization variant in C-level I think LuaRuntime cinit need to be somehow splitted.

An example of your own usage could help to understand how you intend this to work.

When reworked my code I've added Cython proxy module in new commit 32fd682cf621fae7288d840af1be8bd249418546. You could find complete code in embedded.pyx Also I've added code in setup.py to compile it. Compilation produce embedded.cp39-win_amd64.pyd (for win) and this library need to be placed in folder where target Lua interpreter looks for require C-modules and renamed to libpylua.dll (*.so if in Linux). In Lua:

require "libpylua"
python.import("some_package")
python.exec("some_var = some_package.somefunction()")

python.import added in 78f4b16d704d105422d511d51551afb3b66ea833

xthebat avatar Oct 04 '22 06:10 xthebat

it should be provided as a C-level API and not be made publicly available to Python programs.

But all initialization LuaRuntime done in __cinit__ and lua_State* should passed to it for correct initialization also for case external pointer. To hide such initialization variant in C-level I think LuaRuntime.__cinit__ need to be somehow splitted.

Idea: we can add an internal extension type (@cython.internal) that only wraps a lua_State* and can be passed as an optional argument, like this:

cdef class _ExternalLuaState:
    lua_State* lua_state

cdef api runtime_from_lua_state(lua_State* lua_state):
    cdef _ExternalLuaState arg = _ExternalLuaState.__new__(_ExternalLuaState)
    arg.lua_state = lua_state
    return LuaRuntime(lua_state=arg)

scoder avatar Nov 14 '22 10:11 scoder