Repository: panda3d/panda3d (language: python)

Issue:
Debugging code inside a task often crashes the program
## Description
Using the extension Python Debugger in Visual Studio Code, debugging code running in a Python Task is likely to crash the program with the AssertionError `AssertionError: _function != nullptr at line 589 of panda/src/event/pythonTask.cxx`. While this is likely to happen, the crash does not consistently happen, suggesting the presence of some sort of race condition.

## Steps to Reproduce
Using Visual Studio Code and the Python Debugger extension (which can be installed/managed using the Ctrl+Shift+X hotkey), the following code can be run using the "Python Debugger: Python File" run and debug command:

```py
from direct.showbase.ShowBase import ShowBase
from direct.task.TaskManagerGlobal import taskMgr, Task

class GameBase(ShowBase):
    def run(self):
        taskMgr.doMethodLater(1.0, self.update, "update_task")
        super().run()

    def update(self, task):
        print("Hello World")
        return Task.again

game = GameBase()
game.run()
```

By placing a breakpoint on a line in update (ex: `print("Hello World")`) and pressing F5 to continue whenever the program pauses, the program will likely crash within 1-20 continues with either the AssertionError above, or just completely silently. This error is far more likely to happen with more complicated code than the provided snippet, making debugging an actual project impossible.

## Environment
 * Operating system: Windows 10 Professional
 * System architecture: x86 Intel 9th gen
 * Panda3D version: 1.11.0.dev3444
 * Installation method: pip
 * Python version (if using Python): Python 3.12
 * Compiler (if using C++): N/A



Discussion / hints:
Also of note is that when marking `update` with the `async` keyword to turn it into a coroutine and then reproducing the steps to cause the program to crash, it will always silently crash when it does instead of any exception being raised in the output.
Additionally, upon further testing I found that if I include the line `import gc; gc.disable()` at the bottom of the import statements for the snippet above, I cannot reproduce this crash.

I believe there may be some sort of issue with the garbage collection implementation introduced in this commit:
https://github.com/panda3d/panda3d/commit/e5bd00f91f9ef1522f65bd6ebfa23dbe41abdf5e
Disabling the garbage collector actually doesn't seem to completely fix this issue, though it does appear to make it less frequent.
I tried the same code multiple times and it works fine. I use python 3.11 and panda 1.10.14
Commit https://github.com/panda3d/panda3d/commit/e5bd00f91f9ef1522f65bd6ebfa23dbe41abdf5e doesn't exist in 1.10.14 (1.10.14 was released on Jan 8, while the commit was Mar 29), which leads me to believe more that there is something problematic with the commit and that it should likely be reverted for the time being.

Fix patch:
diff --git a/panda/src/event/pythonTask.cxx b/panda/src/event/pythonTask.cxx
index 44b09845594..6e513dbe4cc 100644
--- a/panda/src/event/pythonTask.cxx
+++ b/panda/src/event/pythonTask.cxx
@@ -174,9 +174,15 @@ get_args() {
       PyTuple_SET_ITEM(with_task, i, Py_NewRef(item));
     }
 
-    this->ref();
-    PyObject *self = DTool_CreatePyInstance(this, Dtool_PythonTask, true, false);
-    PyTuple_SET_ITEM(with_task, num_args, self);
+    // 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.
+    if (__self__ == nullptr) {
+      // A __self__ instance does not exist, let's create one now.
+      this->ref();
+      __self__ = DTool_CreatePyInstance(this, Dtool_PythonTask, true, false);
+    }
+
+    PyTuple_SET_ITEM(with_task, num_args, Py_NewRef(__self__));
     return with_task;
   }
   else {
@@ -1004,11 +1010,16 @@ call_owner_method(const char *method_name) {
 void PythonTask::
 call_function(PyObject *function) {
   if (function != Py_None) {
-    this->ref();
-    PyObject *self = DTool_CreatePyInstance(this, Dtool_PythonTask, true, false);
-    PyObject *result = PyObject_CallOneArg(function, self);
+    // 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.
+    if (__self__ == nullptr) {
+      // A __self__ instance does not exist, let's create one now.
+      this->ref();
+      __self__ = DTool_CreatePyInstance(this, Dtool_PythonTask, true, false);
+    }
+
+    PyObject *result = PyObject_CallOneArg(function, __self__);
     Py_XDECREF(result);
-    Py_DECREF(self);
   }
 }
 


Test patch:
diff --git a/tests/event/test_pythontask.py b/tests/event/test_pythontask.py
index 584788494fd..c793067f8b3 100644
--- a/tests/event/test_pythontask.py
+++ b/tests/event/test_pythontask.py
@@ -1,4 +1,4 @@
-from panda3d.core import PythonTask
+from panda3d.core import AsyncTaskManager, PythonTask
 from contextlib import contextmanager
 import pytest
 import types
@@ -115,3 +115,39 @@ def test_pythontask_cycle():
                 break
         else:
             pytest.fail('not found in garbage')
+
+def test_task_persistent_wrapper():
+    task_mgr = AsyncTaskManager.get_global_ptr()
+    task_chain = task_mgr.make_task_chain("test_task_persistent_wrapper")
+
+    # Create a subclass of PythonTask
+    class PythonTaskSubclassTest(PythonTask):
+        pass
+
+    def task_main(task):
+        # Set our result to be the input task we got into this function
+        task.set_result(task)
+        # Verify we got the subclass
+        assert isinstance(task, PythonTaskSubclassTest)
+        return task.done
+
+    done_callback_reached = False
+
+    def done_callback(task):
+        # Verify we got the subclass
+        assert isinstance(task, PythonTaskSubclassTest)
+        nonlocal done_callback_reached
+        done_callback_reached = True
+
+    task = PythonTaskSubclassTest(task_main)
+    task.set_task_chain(task_chain.name)
+    task.set_upon_death(done_callback)
+    task_mgr.add(task)
+    task_chain.wait_for_tasks()
+
+    assert task.done()
+    assert not task.cancelled()
+    # Verify the task passed into the function is the same task we created
+    assert task.result() is task
+    # Verify the done callback worked and was tested
+    assert done_callback_reached