Support cyclic garbage collection through PythonTask objects

dtool/src/interrogate/functionRemap.cxx

@@ -17,6 +17,7 @@
 #include "interrogate.h"
 #include "parameterRemap.h"
 #include "parameterRemapThis.h"
+#include "parameterRemapUnchanged.h"
 #include "interfaceMaker.h"
 #include "interrogateBuilder.h"
 
@@ -447,6 +448,7 @@ get_call_str(const string &container, const vector_string &pexprs) const {
 
     for (pn = _first_true_parameter;
          pn < num_parameters; ++pn) {
+      nassertd(pn < _parameters.size()) break;
       call << separator;
       _parameters[pn]._remap->pass_parameter(call, get_parameter_expr(pn, pexprs));
       separator = ", ";
@@ -590,10 +592,16 @@ setup_properties(const InterrogateFunction &ifunc, InterfaceMaker *interface_mak
     if (param._remap == (ParameterRemap *)NULL) {
       // If we can't handle one of the parameter types, we can't call
       // the function.
-      //nout << "Can't handle parameter " << i << " of method " << *_cppfunc << "\n";
-      return false;
+      if (fname == "__traverse__") {
+        // Hack to record this even though we can't wrap visitproc.
+        param._remap = new ParameterRemapUnchanged(type);
+      } else {
+        //nout << "Can't handle parameter " << i << " of method " << *_cppfunc << "\n";
+        return false;
+      }
+    } else {
+      param._remap->set_default_value(params[i]->_initializer);
     }
-    param._remap->set_default_value(params[i]->_initializer);
 
     if (!param._remap->is_valid()) {
       nout << "Invalid remap for parameter " << i << " of method " << *_cppfunc << "\n";

dtool/src/interrogate/interfaceMaker.cxx

@@ -141,6 +141,12 @@ check_protocols() {
   for (fi = _methods.begin(); fi != _methods.end(); ++fi) {
     Function *func = (*fi);
     flags |= func->_flags;
+
+    if (func->_ifunc.get_name() == "__traverse__") {
+      // If we have a method named __traverse__, we implement Python's
+      // cyclic garbage collection protocol.
+      _protocol_types |= PT_python_gc;
+    }
   }
 
   if ((flags & (FunctionRemap::F_getitem_int | FunctionRemap::F_size)) ==

dtool/src/interrogate/interfaceMaker.h

@@ -155,6 +155,7 @@ public:
       PT_make_copy        = 0x0004,
       PT_copy_constructor = 0x0008,
       PT_iter             = 0x0010,
+      PT_python_gc        = 0x0020,
     };
     int _protocol_types;
   };

dtool/src/interrogate/interfaceMakerPythonNative.cxx

@@ -560,6 +560,18 @@ get_slotted_function_def(Object *obj, Function *func, FunctionRemap *remap,
     return true;
   }
 
+  if (method_name == "__traverse__") {
+    def._answer_location = "tp_traverse";
+    def._wrapper_type = WT_traverse;
+    return true;
+  }
+
+  if (method_name == "__clear__") {
+    def._answer_location = "tp_clear";
+    def._wrapper_type = WT_inquiry;
+    return true;
+  }
+
   if (remap->_type == FunctionRemap::T_typecast_method) {
     // A typecast operator.  Check for a supported low-level typecast type.
     if (TypeManager::is_bool(remap->_return_type->get_orig_type())) {
@@ -671,7 +683,9 @@ get_valid_child_classes(std::map<std::string, CastDetails> &answer, CPPStructTyp
 //
 ///////////////////////////////////////////////////////////////////////////////
 void InterfaceMakerPythonNative::
-write_python_instance(ostream &out, int indent_level, const std::string &return_expr, bool owns_memory, const std::string &class_name, CPPType *ctype, bool is_const) {
+write_python_instance(ostream &out, int indent_level, const string &return_expr,
+                      bool owns_memory, const string &class_name,
+                      CPPType *ctype, bool is_const) {
   out << boolalpha;
 
   if (IsPandaTypedObject(ctype->as_struct_type())) {
@@ -2143,6 +2157,37 @@ write_module_class(ostream &out, Object *obj) {
         }
         break;
 
+      case WT_traverse:
+        // int __traverse__(PyObject *self, visitproc visit, void *arg)
+        // This is a low-level function.  Overloads are not supported.
+        {
+          out << "//////////////////\n";
+          out << "//  A wrapper function to satisfy Python's internal calling conventions.\n";
+          out << "//     " << ClassName << " ..." << rfi->second._answer_location << " = " << methodNameFromCppName(func, export_class_name, false) << "\n";
+          out << "//////////////////\n";
+          out << "static int " << def._wrapper_name << "(PyObject *self, visitproc visit, void *arg) {\n";
+          out << "  " << cClassName << " *local_this = NULL;\n";
+          out << "  DTOOL_Call_ExtractThisPointerForType(self, &Dtool_" << ClassName << ", (void **) &local_this);\n";
+          out << "  if (local_this == NULL) {\n";
+          out << "    PyErr_SetString(PyExc_AttributeError, \"C++ object is not yet constructed, or already destructed.\");\n";
+          out << "    return -1;\n";
+          out << "  }\n\n";
+
+          // Find the remap.  There should be only one.
+          FunctionRemap *remap = func->_remaps.front();
+
+          vector_string params(1);
+          if (remap->_flags & FunctionRemap::F_explicit_self) {
+            params.push_back("self");
+          }
+          params.push_back("visit");
+          params.push_back("arg");
+
+          out << "  return " << remap->call_function(out, 2, false, "local_this", params) << ";\n";
+          out << "}\n\n";
+        }
+        break;
+
       case WT_none:
         // Nothing special about the wrapper function: just write it normally.
         string fname = "static PyObject *" + def._wrapper_name + "(PyObject *self, PyObject *args, PyObject *kwds)\n";
@@ -2585,15 +2630,20 @@ write_module_class(ostream &out, Object *obj) {
     out << "    0, // tp_as_buffer\n";
   }
 
+  string gcflag;
+  if (obj->_protocol_types & Object::PT_python_gc) {
+    gcflag = " | Py_TPFLAGS_HAVE_GC";
+  }
+
   // long tp_flags;
   if (has_local_getbuffer) {
     out << "#if PY_VERSION_HEX >= 0x02060000\n";
-    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES | Py_TPFLAGS_HAVE_NEWBUFFER,\n";
+    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES | Py_TPFLAGS_HAVE_NEWBUFFER" << gcflag << ",\n";
     out << "#else\n";
-    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES,\n";
+    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES" << gcflag << ",\n";
     out << "#endif\n";
   } else {
-    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES,\n";
+    out << "    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_CHECKTYPES" << gcflag << ",\n";
   }
 
   // const char *tp_doc;
@@ -2668,7 +2718,11 @@ write_module_class(ostream &out, Object *obj) {
   // newfunc tp_new;
   out << "    Dtool_new_" << ClassName << ",\n";
   // freefunc tp_free;
-  out << "    PyObject_Del,\n";
+  if (obj->_protocol_types & Object::PT_python_gc) {
+    out << "    PyObject_GC_Del,\n";
+  } else {
+    out << "    PyObject_Del,\n";
+  }
   // inquiry tp_is_gc;
   out << "    0, // tp_is_gc\n";
   // PyObject *tp_bases;
@@ -4625,14 +4679,8 @@ write_function_instance(ostream &out, FunctionRemap *remap,
         format_specifiers += "h";
         parameter_list += ", &" + param_name;
       }
-
-      extra_convert += "PyObject *" + param_name + "_long = PyNumber_Long(" + param_name + ");";
-      extra_param_check += " && " + param_name + "_long != NULL";
-      pexpr_string = "(" + type->get_local_name(&parser) + ")" +
-                     "PyLong_AsLongLong(" + param_name + "_long)";
-      extra_cleanup += "Py_XDECREF(" + param_name + "_long);";
-      expected_params += "long long";
-      ++num_params;
+      expected_params += "int";
+      only_pyobjects = false;
 
     } else if (TypeManager::is_unsigned_integer(type)) {
       if (args_type == AT_single_arg) {

dtool/src/interrogate/interfaceMakerPythonNative.h

@@ -80,6 +80,7 @@ private:
     WT_ternary_operator,
     WT_inplace_binary_operator,
     WT_inplace_ternary_operator,
+    WT_traverse,
   };
 
   // This enum is passed to the wrapper generation functions to indicate

dtool/src/parser-inc/Python.h

@@ -51,4 +51,6 @@ PyObject _Py_FalseStruct;
 // This file defines PY_VERSION_HEX, which is used in some places.
 #include "patchlevel.h"
 
+typedef void *visitproc;
+
 #endif  // PYTHON_H

panda/src/event/pythonTask.cxx

@@ -54,14 +54,8 @@ PythonTask(PyObject *function, const string &name) :
   __dict__ = PyDict_New();
 
 #ifndef SIMPLE_THREADS
-  // Ensure that the Python threading system is initialized and ready
-  // to go.
+  // Ensure that the Python threading system is initialized and ready to go.
 #ifdef WITH_THREAD  // This symbol defined within Python.h
-
-#if PY_VERSION_HEX >= 0x03020000
-  Py_Initialize();
-#endif
-
   PyEval_InitThreads();
 #endif
 #endif
@@ -358,6 +352,38 @@ __getattr__(PyObject *attr) const {
   return item;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: PythonTask::__traverse__
+//       Access: Published
+//  Description: Called by Python to implement cycle detection.
+////////////////////////////////////////////////////////////////////
+int PythonTask::
+__traverse__(visitproc visit, void *arg) {
+  Py_VISIT(_function);
+  Py_VISIT(_args);
+  Py_VISIT(_upon_death);
+  Py_VISIT(_owner);
+  Py_VISIT(__dict__);
+  Py_VISIT(_generator);
+  return 0;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: PythonTask::__clear__
+//       Access: Published
+//  Description: Called by Python to implement cycle breaking.
+////////////////////////////////////////////////////////////////////
+int PythonTask::
+__clear__() {
+  Py_CLEAR(_function);
+  Py_CLEAR(_args);
+  Py_CLEAR(_upon_death);
+  Py_CLEAR(_owner);
+  Py_CLEAR(__dict__);
+  Py_CLEAR(_generator);
+  return 0;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: PythonTask::is_runnable
 //       Access: Protected, Virtual

panda/src/event/pythonTask.h

@@ -20,6 +20,8 @@
 #include "asyncTask.h"
 
 #ifdef HAVE_PYTHON
+#include "py_panda.h"
+
 ////////////////////////////////////////////////////////////////////
 //       Class : PythonTask
 // Description : This class exists to allow association of a Python
@@ -47,6 +49,9 @@ PUBLISHED:
   int __delattr__(PyObject *self, PyObject *attr);
   PyObject *__getattr__(PyObject *attr) const;
 
+  int __traverse__(visitproc visit, void *arg);
+  int __clear__();
+
   INLINE void set_delay(PyObject *delay);
   INLINE PyObject *get_delay() const;