Try to solve deadlock when deallocating Python objects in threads

[scoder]

Closes #250

[astoff]

I suppose the issue here is in the test itself, as this change makes it pass on my local machine:

modified   lupa/tests/test.py
@@ -103,14 +103,13 @@ class TestLuaRuntimeRefcounting(LupaTestCase):
         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():
+            lua = self.lupa.LuaRuntime()
             ref = [lua.eval("{}")]
 
             def trigger_gc(ref):

[astoff]

@scoder Did you get the chance to try out my previous comment from last month?

[scoder]

I had looked at it when you wrote it and wasn't sure. The difference is that, in your proposal, the Lua runtime gets cleaned up after each run, thus clearing all objects and freeing all memory. It's good to know that that doesn't leave anything behind, but my implementation wanted to make sure that the objects get cleaned up as early as possible (and safe), preferably before returning from Lua to Python, and not just kept around unnecessarily. Your change doesn't test that any more.

I still think there's something wrong (or hopefully only missing) in my implementation.

[scoder]

I took another look and it turns out that most of the excess references in the test are cleaned up by triggering a run of the Lua garbage collector. Most, but not all. There's around 20 remaining references that are created by the test and not cleaned up afterwards:

[(<cell at 0x7fbd0f040bb0: dict object at 0x7fbd0f1d2500>,),
 <function TestLuaRuntimeRefcounting._run_gc_test.<locals>.<listcomp> at 0x7fbd0f0b8790>,
 <lupa.luajit20._PyReference object at 0x7fbd0f041030>,
 <weakref at 0x7fbd0f0df7e0; to 'Thread' at 0x7fbd0f0408e0>,
 <lupa.luajit20._PyReference object at 0x7fbd0f040a00>,
 <Thread(Thread-707 (trigger_gc), stopped 140449962329664)>,
 {'_daemonic': False,
  '_ident': 140449962329664,
  '_initialized': True,
  '_invoke_excepthook': <function _make_invoke_excepthook.<locals>.invoke_excepthook at 0x7fbd0ef500d0>,
  '_is_stopped': True,
  '_name': 'Thread-707 (trigger_gc)',
  '_native_id': 134030,
  '_started': <threading.Event object at 0x7fbd0f040f40>,
  '_stderr': <_io.TextIOWrapper name='<stderr>' mode='w' encoding='utf-8'>,
  '_tstate_lock': None},
 <threading.Event object at 0x7fbd0f040f40>,
 <function _make_invoke_excepthook.<locals>.invoke_excepthook at 0x7fbd0ef500d0>,
 {'_cond': <Condition(<unlocked _thread.lock object at 0x7fbd0f1e3e40>, 0)>, '_flag': True},
 (<cell at 0x7fbd0f0407f0: builtin_function_or_method object at 0x7fbd0f6b9530>,
  <cell at 0x7fbd0f040490: module object at 0x7fbd0f6a6390>,
  <cell at 0x7fbd0f041150: builtin_function_or_method object at 0x7fbd0f6d6110>,
  <cell at 0x7fbd0f0410c0: function object at 0x7fbd0f5afc70>,
  <cell at 0x7fbd0f040b20: builtin_function_or_method object at 0x7fbd0f6a66b0>),
 <Condition(<unlocked _thread.lock object at 0x7fbd0f1e3e40>, 0)>,
 <cell at 0x7fbd0f040b20: builtin_function_or_method object at 0x7fbd0f6a66b0>,
 <cell at 0x7fbd0f0410c0: function object at 0x7fbd0f5afc70>,
 <cell at 0x7fbd0f041150: builtin_function_or_method object at 0x7fbd0f6d6110>,
 <cell at 0x7fbd0f040490: module object at 0x7fbd0f6a6390>,
 <cell at 0x7fbd0f0407f0: builtin_function_or_method object at 0x7fbd0f6b9530>,
 {'_lock': <unlocked _thread.lock object at 0x7fbd0f1e3e40>,
  '_waiters': deque([]),
  'acquire': <built-in method acquire of _thread.lock object at 0x7fbd0f1e3e40>,
  'release': <built-in method release of _thread.lock object at 0x7fbd0f1e3e40>},
 <unlocked _thread.lock object at 0x7fbd0f1e3e40>,
 <built-in method acquire of _thread.lock object at 0x7fbd0f1e3e40>,
 <built-in method release of _thread.lock object at 0x7fbd0f1e3e40>,
 deque([])]

The test passes the Thread object through Lua and it's not obvious if some of these are related to that.

[astoff]

my implementation wanted to make sure that the objects get cleaned up as early as possible (and safe),

I don't know much about GC, but isn't this due to Lua not using a reference count approach? What I imagine is, with reference counts you can deallocate objects as soon as the count reaches 0, but with other GC approaches every object survives at least until the next GC cycle runs.

... preferably before returning from Lua to Python

In other words, in Lua you wouldn't expect that GC always runs before returning to Python.

Anyway, this is just a wild hunch. I went through the old version of this PR and couldn't spot anything missing in it.

[scoder]

I think it's too difficult to test correctly for reference leaks here, so the test sadly has to go without that. I simplified it.