From b63d5fbe8cc18f746301c14294aa43193b3c2afb Mon Sep 17 00:00:00 2001 From: rdb Date: Sun, 15 Mar 2015 12:24:06 +0100 Subject: [PATCH] Support cyclic garbage collection through PythonTask objects --- dtool/src/interrogate/functionRemap.cxx | 14 +++- dtool/src/interrogate/interfaceMaker.cxx | 6 ++ dtool/src/interrogate/interfaceMaker.h | 1 + .../interfaceMakerPythonNative.cxx | 74 +++++++++++++++---- .../interrogate/interfaceMakerPythonNative.h | 1 + dtool/src/parser-inc/Python.h | 2 + panda/src/event/pythonTask.cxx | 40 ++++++++-- panda/src/event/pythonTask.h | 5 ++ 8 files changed, 120 insertions(+), 23 deletions(-) diff --git a/dtool/src/interrogate/functionRemap.cxx b/dtool/src/interrogate/functionRemap.cxx index 09aa90f7e9..ea09c472e7 100644 --- a/dtool/src/interrogate/functionRemap.cxx +++ b/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"; diff --git a/dtool/src/interrogate/interfaceMaker.cxx b/dtool/src/interrogate/interfaceMaker.cxx index 61837f9c34..03398895b8 100644 --- a/dtool/src/interrogate/interfaceMaker.cxx +++ b/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)) == diff --git a/dtool/src/interrogate/interfaceMaker.h b/dtool/src/interrogate/interfaceMaker.h index 021cb608bf..71a1f0eb6d 100644 --- a/dtool/src/interrogate/interfaceMaker.h +++ b/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; }; diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index 366c2ea05c..971a5ef24d 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/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 &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) { diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.h b/dtool/src/interrogate/interfaceMakerPythonNative.h index 0e27ca3166..7288f8ce57 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.h +++ b/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 diff --git a/dtool/src/parser-inc/Python.h b/dtool/src/parser-inc/Python.h index 8e4465b602..de17409f53 100755 --- a/dtool/src/parser-inc/Python.h +++ b/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 diff --git a/panda/src/event/pythonTask.cxx b/panda/src/event/pythonTask.cxx index 3a4573585f..a5ea42c1a7 100644 --- a/panda/src/event/pythonTask.cxx +++ b/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 diff --git a/panda/src/event/pythonTask.h b/panda/src/event/pythonTask.h index d9990e92a4..999b5e6454 100644 --- a/panda/src/event/pythonTask.h +++ b/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;