task: Implement garbage collector support for PythonTask

This adds persistent wrapper support (introduced by the previous commit) to PythonTask, which makes it possible for reference cycles involving tasks to be found and destroyed.

The major caveat is that it always creates a reference cycle.  This can be broken automatically if there is no more Python reference to it by the time the last C++ reference is dropped, but the other way around requires the garbage collector.

For tasks, I think this it is generally the case that the last reference is in C++, since tasks are usually created and then handed off to the C++ task manager, and for applications that don't want to rely on the GC, it is easy to work around.  If this turns out to be a problem, though, we can add a special garbage collection pass to the task manager.

Author: rdb

panda/src/event/pythonTask.cxx

@@ -97,9 +97,17 @@ PythonTask::
     _exc_traceback = nullptr;
   }
 
+  PyObject *self = __self__;
+  if (self != nullptr) {
+    PyObject_GC_UnTrack(self);
+    __self__ = nullptr;
+    Py_DECREF(self);
+  }
+
+  // All of these may have already been cleared by __clear__.
   Py_XDECREF(_function);
-  Py_DECREF(_args);
-  Py_DECREF(__dict__);
+  Py_XDECREF(_args);
+  Py_XDECREF(__dict__);
   Py_XDECREF(_exception);
   Py_XDECREF(_exc_value);
   Py_XDECREF(_exc_traceback);
@@ -364,14 +372,13 @@ __getattribute__(PyObject *self, PyObject *attr) const {
  */
 int PythonTask::
 __traverse__(visitproc visit, void *arg) {
-/*
+  Py_VISIT(__self__);
   Py_VISIT(_function);
   Py_VISIT(_args);
   Py_VISIT(_upon_death);
   Py_VISIT(_owner);
   Py_VISIT(__dict__);
   Py_VISIT(_generator);
-*/
   return 0;
 }
 
@@ -380,17 +387,58 @@ __traverse__(visitproc visit, void *arg) {
  */
 int PythonTask::
 __clear__() {
-/*
   Py_CLEAR(_function);
   Py_CLEAR(_args);
   Py_CLEAR(_upon_death);
   Py_CLEAR(_owner);
   Py_CLEAR(__dict__);
   Py_CLEAR(_generator);
-*/
+
+  Py_CLEAR(__self__);
   return 0;
 }
 
+/**
+ *
+ */
+bool PythonTask::
+unref() const {
+  if (!AsyncTask::unref()) {
+    // It was cleaned up by the Python garbage collector.
+    return false;
+  }
+
+  // If the last reference to the object is the one being held by Python,
+  // check whether the Python wrapper itself is also at a refcount of 1.
+  bool result = true;
+  if (get_ref_count() == 1) {
+    PyGILState_STATE gstate = PyGILState_Ensure();
+
+    // Check whether we have a Python wrapper.  This is not the case if the
+    // object has been created by C++ and never been exposed to Python code.
+    PyObject *self = __self__;
+    if (self != nullptr) {
+      int ref_count = Py_REFCNT(self);
+      assert(ref_count > 0);
+      if (ref_count == 1) {
+        // The last reference to the Python wrapper is being held by us.
+        // Break the reference cycle and allow the object to go away.
+        if (!AsyncTask::unref()) {
+          PyObject_GC_UnTrack(self);
+          ((Dtool_PyInstDef *)self)->_memory_rules = false;
+          __self__ = nullptr;
+          Py_DECREF(self);
+
+          // Let the caller destroy the object.
+          result = false;
+        }
+      }
+    }
+    PyGILState_Release(gstate);
+  }
+  return result;
+}
+
 /**
  * Cancels this task.  This is equivalent to remove(), except for coroutines,
  * for which it will throw an exception into any currently pending await.

panda/src/event/pythonTask.h

@@ -89,6 +89,14 @@ class PythonTask final : public AsyncTask {
   // custom variables may be stored.
   PyObject *__dict__;
 
+public:
+  // This holds a reference to the Python wrapper corresponding to this
+  // PythonTask object.  It is necessary for this to remain consistent for
+  // the __traverse__ method to work correctly.
+  mutable PyObject *__self__ = nullptr;
+
+  virtual bool unref() const;
+
 protected:
   virtual bool cancel();
 

tests/event/test_pythontask.py

@@ -1,7 +1,26 @@
 from panda3d.core import PythonTask
+from contextlib import contextmanager
 import pytest
 import types
 import sys
+import gc
+
+
+@contextmanager
+def gc_disabled():
+    gc.disable()
+    gc.collect()
+    gc.freeze()
+    gc.set_debug(gc.DEBUG_SAVEALL)
+
+    try:
+        yield
+    finally:
+        gc.set_debug(0)
+        gc.garbage.clear()
+        gc.unfreeze()
+        gc.collect()
+        gc.enable()
 
 
 def test_pythontask_property_builtin():
@@ -76,3 +95,23 @@ def test_pythontask_dict_set():
     rc2 = sys.getrefcount(d)
 
     assert rc1 == rc2
+
+
+def test_pythontask_cycle():
+    with gc_disabled():
+        task = PythonTask()
+        assert gc.is_tracked(task)
+        task.marker = 'test_pythontask_cycle'
+        task.prop = task
+
+        del task
+
+        gc.collect()
+        assert len(gc.garbage) > 0
+
+        for g in gc.garbage:
+            if isinstance(g, PythonTask) and \
+               getattr(g, 'marker', None) == 'test_pythontask_cycle':
+                break
+        else:
+            pytest.fail('not found in garbage')