diff --git a/panda/src/event/pythonTask.cxx b/panda/src/event/pythonTask.cxx index 6ea95be752..02e9185090 100644 --- a/panda/src/event/pythonTask.cxx +++ b/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. diff --git a/panda/src/event/pythonTask.h b/panda/src/event/pythonTask.h index 88bee56bd9..148252e999 100644 --- a/panda/src/event/pythonTask.h +++ b/panda/src/event/pythonTask.h @@ -89,6 +89,14 @@ PUBLISHED: // 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(); diff --git a/tests/event/test_pythontask.py b/tests/event/test_pythontask.py index d6806b4f72..584788494f 100644 --- a/tests/event/test_pythontask.py +++ b/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')