Skip to content

Commit

Permalink
Try to solve deadlock when deallocating Python objects in threads.
Browse files Browse the repository at this point in the history
Closes #250
  • Loading branch information
scoder committed Jan 28, 2024
1 parent 6ea4992 commit a0c8c1d
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 14 deletions.
63 changes: 49 additions & 14 deletions lupa/_lupa.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ cdef class LuaRuntime:
cdef FastRLock _lock
cdef dict _pyrefs_in_lua
cdef tuple _raised_exception
cdef list _pending_unrefs
cdef bytes _encoding
cdef bytes _source_encoding
cdef object _attribute_filter
Expand Down Expand Up @@ -319,6 +320,28 @@ cdef class LuaRuntime:
if self._memory_status.limit == <size_t> -1:
self._memory_status.limit -= 1

@cython.final
cdef void add_pending_unref(self, int ref) noexcept:
pyval: object = ref
if self._pending_unrefs is None:
self._pending_unrefs = [pyval]
else:
self._pending_unrefs.append(pyval)

@cython.final
cdef int clean_up_pending_unrefs(self) except -1:
if self._pending_unrefs is None or self._state is NULL:
return 0

cdef int ref
L = self._state
pending_unrefs = self._pending_unrefs
self._pending_unrefs = None
for ref in pending_unrefs:
lua.luaL_unref(L, lua.LUA_REGISTRYINDEX, ref)
print(f"Cleaned up {len(pending_unrefs)} Lua refs") # TODO: remove
return 0

def __dealloc__(self):
if self._state is not NULL:
lua.lua_close(self._state)
Expand Down Expand Up @@ -793,8 +816,8 @@ cdef tuple _fix_args_kwargs(tuple args):
################################################################################
# fast, re-entrant runtime locking

cdef inline bint lock_runtime(LuaRuntime runtime) noexcept with gil:
return lock_lock(runtime._lock, pythread.PyThread_get_thread_ident(), True)
cdef inline bint lock_runtime(LuaRuntime runtime, bint blocking=True) noexcept with gil:
return lock_lock(runtime._lock, pythread.PyThread_get_thread_ident(), blocking=blocking)

cdef inline void unlock_runtime(LuaRuntime runtime) noexcept nogil:
unlock_lock(runtime._lock)
Expand Down Expand Up @@ -822,15 +845,20 @@ cdef class _LuaObject:
def __dealloc__(self):
if self._runtime is None:
return
runtime = self._runtime
self._runtime = None
ref = self._ref
if ref == lua.LUA_NOREF:
return
self._ref = lua.LUA_NOREF
cdef lua_State* L = self._state
if L is not NULL and self._ref != lua.LUA_NOREF:
locked = lock_runtime(self._runtime)
lua.luaL_unref(L, lua.LUA_REGISTRYINDEX, self._ref)
self._ref = lua.LUA_NOREF
runtime = self._runtime
self._runtime = None
if L is not NULL:
locked = lock_runtime(runtime, blocking=False)
if locked:
lua.luaL_unref(L, lua.LUA_REGISTRYINDEX, ref)
unlock_runtime(runtime)
return
runtime.add_pending_unref(ref)

@cython.final
cdef inline int push_lua_object(self, lua_State* L) except -1:
Expand Down Expand Up @@ -1288,15 +1316,20 @@ cdef class _LuaIter:
def __dealloc__(self):
if self._runtime is None:
return
runtime = self._runtime
self._runtime = None
ref = self._refiter
if self._refiter == lua.LUA_NOREF:
return
self._refiter = lua.LUA_NOREF
cdef lua_State* L = self._state
if L is not NULL and self._refiter != lua.LUA_NOREF:
locked = lock_runtime(self._runtime)
lua.luaL_unref(L, lua.LUA_REGISTRYINDEX, self._refiter)
self._refiter = lua.LUA_NOREF
runtime = self._runtime
self._runtime = None
if L is not NULL:
locked = lock_runtime(runtime, blocking=False)
if locked:
lua.luaL_unref(L, lua.LUA_REGISTRYINDEX, ref)
unlock_runtime(runtime)
return
runtime.add_pending_unref(ref)

def __repr__(self):
return u"LuaIter(%r)" % (self._obj)
Expand Down Expand Up @@ -1829,6 +1862,7 @@ cdef object execute_lua_call(LuaRuntime runtime, lua_State *L, Py_ssize_t nargs)
result_status = lua.lua_pcall(L, nargs, lua.LUA_MULTRET, has_lua_traceback_func)
if has_lua_traceback_func:
lua.lua_remove(L, 1)
runtime.clean_up_pending_unrefs()
results = unpack_lua_results(runtime, L)
if result_status:
if isinstance(results, BaseException):
Expand Down Expand Up @@ -2028,6 +2062,7 @@ cdef bint call_python(LuaRuntime runtime, lua_State *L, py_object* py_obj) excep
lua.lua_settop(L, 0) # FIXME
result = f(*args, **kwargs)

runtime.clean_up_pending_unrefs()
return py_function_result_to_lua(runtime, L, result)

cdef int py_call_with_gil(lua_State* L, py_object *py_obj) noexcept with gil:
Expand Down
22 changes: 22 additions & 0 deletions lupa/tests/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ def get_attr(obj, name):
# Seems related to running the test twice in the same Lupa module?
self._run_gc_test(make_refcycle, off_by_one=True)

def test_lupa_gc_deadlock(self):
lua = self.lupa.LuaRuntime()

def assert_no_deadlock(thread):
thread.start()
thread.join(1)
assert not thread.is_alive(), "thread didn't finish - deadlock?"

def delete_table_reference_in_thread():
ref = [lua.eval("{}")]

def trigger_gc(ref):
del ref[0]

lua.execute(
"f,x=...; f(x)",
assert_no_deadlock,
threading.Thread(target=trigger_gc, args=[ref]),
)

self._run_gc_test(delete_table_reference_in_thread)


class TestLuaRuntime(SetupLuaRuntimeMixin, LupaTestCase):
def test_lua_version(self):
Expand Down

0 comments on commit a0c8c1d

Please sign in to comment.