diff --git a/dtool/src/interrogate/functionRemap.cxx b/dtool/src/interrogate/functionRemap.cxx index 6d6ce0f911..b692577067 100644 --- a/dtool/src/interrogate/functionRemap.cxx +++ b/dtool/src/interrogate/functionRemap.cxx @@ -450,8 +450,6 @@ get_call_str(const string &container, const vector_string &pexprs) const { _parameters[pn]._remap->pass_parameter(call, get_parameter_expr(pn, pexprs)); } else { - const char *separator = ""; - // If this function is marked as having an extension function, call that // instead. if (_extension) { @@ -488,33 +486,41 @@ get_call_str(const string &container, const vector_string &pexprs) const { } } call << "("; - - if (_flags & F_explicit_self) { - // Pass on the PyObject * that we stripped off above. - call << separator << "self"; - separator = ", "; - } - if (_flags & F_explicit_cls) { - call << separator << "cls"; - separator = ", "; - } - - size_t pn; - size_t num_parameters = pexprs.size(); - - 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 = ", "; - } + write_call_args(call, pexprs); call << ")"; } return call.str(); } +/** + * Writes the arguments to pass to the function. + */ +void FunctionRemap:: +write_call_args(std::ostream &call, const vector_string &pexprs) const { + const char *separator = ""; + if (_flags & F_explicit_self) { + // Pass on the PyObject * that we stripped off above. + call << separator << "self"; + separator = ", "; + } + if (_flags & F_explicit_cls) { + call << separator << "cls"; + separator = ", "; + } + + size_t pn; + size_t num_parameters = pexprs.size(); + + 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 = ", "; + } +} + /** * Returns the minimum number of arguments that needs to be passed to this * function. diff --git a/dtool/src/interrogate/functionRemap.h b/dtool/src/interrogate/functionRemap.h index f6d2fea30c..e662e40b75 100644 --- a/dtool/src/interrogate/functionRemap.h +++ b/dtool/src/interrogate/functionRemap.h @@ -61,6 +61,7 @@ public: FunctionWrapperIndex make_wrapper_entry(FunctionIndex function_index); std::string get_call_str(const std::string &container, const vector_string &pexprs) const; + void write_call_args(std::ostream &out, const vector_string &pexprs) const; int get_min_num_args() const; int get_max_num_args() const; diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.cxx b/dtool/src/interrogate/interfaceMakerPythonNative.cxx index dbdfdb8601..d10722f68a 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.cxx +++ b/dtool/src/interrogate/interfaceMakerPythonNative.cxx @@ -1106,6 +1106,170 @@ write_class_details(ostream &out, Object *obj) { std::string ClassName = make_safe_name(obj->_itype.get_scoped_name()); std::string cClassName = obj->_itype.get_true_name(); + CPPType *cpptype = TypeManager::resolve_type(obj->_itype._cpptype); + CPPStructType *struct_type = cpptype->as_struct_type(); + + // Determine which external imports we will need. + std::map details; + std::map::iterator di; + builder.get_type(TypeManager::unwrap(cpptype), false); + get_valid_child_classes(details, cpptype->as_struct_type()); + for (di = details.begin(); di != details.end(); di++) { + // InterrogateType ptype =idb->get_type(di->first); + if (di->second._is_legal_py_class && !isExportThisRun(di->second._structType)) { + _external_imports.insert(TypeManager::resolve_type(di->second._structType)); + } + // out << "IMPORT_THIS struct Dtool_PyTypedObject Dtool_" << + // make_safe_name(di->second._to_class_name) << ";\n"; + } + + bool py_subclassable = is_python_subclassable(struct_type); + if (py_subclassable) { + assert(!obj->_constructors.empty()); + out << "/**\n"; + out << " * Proxy class for " << cClassName << "\n"; + out << " */\n"; + out << "class DtoolProxy_" << ClassName << " final : public " << cClassName << ", public DtoolProxy {\n"; + out << "public:\n"; + // I'd love just to use "using" to inherit the constructors from the base. + // However, MSVC seems to have a bug inheriting constructors with default + // arguments. + if (struct_type->is_default_constructible()) { + out << " DtoolProxy_" << ClassName << "() = default;\n"; + } + for (Function *func : obj->_constructors) { + for (FunctionRemap *remap : func->_remaps) { + CPPParameterList::Parameters ¶ms = remap->_ftype->_parameters->_parameters; + if (params.empty()) { + continue; + } + out << " DtoolProxy_" << ClassName << "("; + for (size_t i = 0; i < params.size(); ++i) { + if (i > 0) { + out << ", "; + } + params[i]->_type->output_instance(out, remap->get_parameter_name(i), &parser); + if (i > 0 && params[i]->_initializer != nullptr) { + out << " = " << *params[i]->_initializer; + } + } + out << ")"; + if (remap->_ftype->_flags & CPPFunctionType::F_noexcept) { + out << " noexcept"; + } + out << " : " << cClassName << "("; + for (size_t i = 0; i < params.size(); ++i) { + if (i > 0) { + out << ", "; + } + // Determine whether to pass with move semantics or not. + CPPType *param_type = params[i]->_type; + CPPType::SubType subtype = param_type->get_subtype(); + while (subtype == CPPDeclaration::ST_typedef) { + param_type = param_type->as_typedef_type()->_type; + subtype = param_type->get_subtype(); + } + if (subtype == CPPType::ST_pointer || + subtype == CPPType::ST_const || + subtype == CPPType::ST_enum || + subtype == CPPType::ST_simple || + (subtype == CPPType::ST_reference && param_type->as_reference_type()->_value_category == CPPReferenceType::VC_lvalue)) { + out << remap->get_parameter_name(i); + } else { + out << "std::move(" << remap->get_parameter_name(i) << ")"; + } + } + out << ") {}\n"; + } + } + + // Move constructor is never listed above, but we do use it. + if (struct_type->is_move_constructible()) { + out << " DtoolProxy_" << ClassName << "(" << cClassName << " &&from)"; + CPPInstance *ctor = struct_type->get_move_constructor(); + if (ctor != nullptr) { + CPPFunctionType *ftype = ctor->_type->as_function_type(); + if (ftype != nullptr && (ftype->_flags & CPPFunctionType::F_noexcept) != 0) { + out << " noexcept"; + } + } + out << " : " << cClassName << "(std::move(from)) {}\n"; + } + out << "\n"; + out << " virtual bool unref() const override {\n"; + out << " if (!(" << cClassName << "::unref())) {\n"; + out << " // It was cleaned up by the Python garbage collector.\n"; + out << " return false;\n"; + out << " }\n"; + out << " nassertr(DtoolProxy::_self != nullptr, true);\n"; + out << "\n"; + out << " // If the last reference to the object is the one being held by Python,\n"; + out << " // check whether the Python wrapper itself is also at a refcount of 1.\n"; + out << " bool result = true;\n"; + out << " if (get_ref_count() == 1) {\n"; + out << " PyGILState_STATE gstate = PyGILState_Ensure();\n"; + out << " int ref_count = Py_REFCNT(DtoolProxy::_self);\n"; + out << " assert(ref_count > 0);\n"; + out << " if (ref_count == 1) {\n"; + out << " // The last reference to the Python wrapper is being held by us.\n"; + out << " // Break the reference cycle and allow the object to go away.\n"; + out << " if (!" << cClassName << "::unref()) {\n"; + out << " PyObject_GC_UnTrack(DtoolProxy::_self);\n"; + out << " ((Dtool_PyInstDef *)DtoolProxy::_self)->_memory_rules = false;\n"; + out << " Py_CLEAR(DtoolProxy::_self);\n"; + out << "\n"; + out << " // Let the caller destroy the object.\n"; + out << " result = false;\n"; + out << " }\n"; + out << " }\n"; + //out << " else {\n"; + //out << " // There is still a Python reference. Make sure it can be cleaned up\n"; + //out << " // by the garbage collector when it gets to it.\n"; + //out << " if (!PyObject_GC_IsTracked(DtoolProxy::_self)) {\n"; + //out << " PyObject_GC_Track(DtoolProxy::_self);\n"; + //out << " }\n"; + //out << " }\n"; + out << " PyGILState_Release(gstate);\n"; + out << " }\n"; + out << " return result;\n"; + out << " }\n"; + out << "\n"; + out << " virtual TypeHandle get_type() const override {\n"; + out << " return DtoolProxy::_type;\n"; + out << " }\n"; + out << "\n"; + out << " virtual TypeHandle force_init_type() override {\n"; + out << " return DtoolProxy::_type;\n"; + out << " }\n"; + out << "};\n"; + out << "\n"; + + out << "static PyObject *Dtool_WrapProxy_" << ClassName << "(void *from_this, PyTypeObject *from_type) {\n"; + out << " DtoolProxy_" << ClassName << " *to_this;\n"; + out << " if (from_type == nullptr || from_type == &Dtool_" << ClassName << "._PyType) {\n"; + out << " to_this = (DtoolProxy_" << ClassName << " *)(" << cClassName << "*)from_this;\n"; + out << " }\n"; + for (di = details.begin(); di != details.end(); di++) { + if (di->second._can_downcast && di->second._is_legal_py_class) { + out << " else if (from_type == (PyTypeObject *)Dtool_Ptr_" << make_safe_name(di->second._to_class_name) << ") {\n"; + out << " " << di->second._to_class_name << " *other_this = (" << di->second._to_class_name << " *)from_this;\n" ; + out << " to_this = (DtoolProxy_" << ClassName << " *)(" << cClassName << " *)other_this;\n"; + out << " }\n"; + } + } + out << " else {\n"; + out << " return nullptr;\n"; + out << " }\n"; + out << " nassertr(to_this->DtoolProxy::_self != nullptr, nullptr);\n"; + out << " PyObject *result = Py_NewRef(to_this->DtoolProxy::_self);\n"; + // Directly call the parent class' unref(), because we know our version + // would only find that the Python refcount > 1 and do nothing else + out << " bool nonzero = to_this->" << cClassName << "::unref();\n"; + out << " nassertr(nonzero, result);\n"; + out << " return result;\n"; + out << "}\n\n"; + } + out << "/**\n"; out << " * Python wrappers for functions of class " << cClassName << "\n" ; out << " */\n"; @@ -1149,11 +1313,9 @@ write_class_details(ostream &out, Object *obj) { } } - CPPType *cpptype = TypeManager::resolve_type(obj->_itype._cpptype); - // If we have "coercion constructors", write a single wrapper to consolidate // those. - int has_coerce = has_coerce_constructor(cpptype->as_struct_type()); + int has_coerce = has_coerce_constructor(struct_type); if (has_coerce > 0) { write_coerce_constructor(out, obj, true); if (has_coerce > 1 && TypeManager::is_reference_count(obj->_itype._cpptype)) { @@ -1176,20 +1338,6 @@ write_class_details(ostream &out, Object *obj) { } } - // Determine which external imports we will need. - std::map details; - std::map::iterator di; - builder.get_type(TypeManager::unwrap(cpptype), false); - get_valid_child_classes(details, cpptype->as_struct_type()); - for (di = details.begin(); di != details.end(); di++) { - // InterrogateType ptype =idb->get_type(di->first); - if (di->second._is_legal_py_class && !isExportThisRun(di->second._structType)) { - _external_imports.insert(TypeManager::resolve_type(di->second._structType)); - } - // out << "IMPORT_THIS struct Dtool_PyTypedObject Dtool_" << - // make_safe_name(di->second._to_class_name) << ";\n"; - } - // Write support methods to cast from and to pointers of this type. { out << "static void *Dtool_UpcastInterface_" << ClassName << "(PyObject *self, Dtool_PyTypedObject *requested_type) {\n"; @@ -1241,27 +1389,46 @@ write_class_details(ostream &out, Object *obj) { out << "static PyObject *Dtool_Wrap_" << ClassName << "(void *from_this, PyTypeObject *from_type) {\n"; out << " " << cClassName << " *to_this;\n"; out << " if (from_type == nullptr || from_type == &Dtool_" << ClassName << "._PyType) {\n"; - out << " to_this = (" << cClassName << "*)from_this;\n"; + out << " to_this = (" << cClassName << " *)from_this;\n"; out << " }\n"; for (di = details.begin(); di != details.end(); di++) { if (di->second._can_downcast && di->second._is_legal_py_class) { out << " else if (from_type == (PyTypeObject *)Dtool_Ptr_" << make_safe_name(di->second._to_class_name) << ") {\n"; out << " " << di->second._to_class_name << " *other_this = (" << di->second._to_class_name << " *)from_this;\n" ; - out << " to_this = (" << cClassName << "*)other_this;\n"; + out << " to_this = (" << cClassName << " *)other_this;\n"; out << " }\n"; } } out << " else {\n"; out << " return nullptr;\n"; out << " }\n"; - out << " // Allocate a new Python instance\n"; - out << " Dtool_PyInstDef *self = (Dtool_PyInstDef *)PyType_GenericAlloc(&Dtool_" << ClassName << "._PyType, 0);\n"; - out << " self->_signature = PY_PANDA_SIGNATURE;\n"; - out << " self->_My_Type = &Dtool_" << ClassName << ";\n"; - out << " self->_ptr_to_object = to_this;\n"; - out << " self->_memory_rules = false;\n"; - out << " self->_is_const = false;\n"; - out << " return (PyObject *)self;\n"; + if (has_self_member(struct_type)) { + out << " PyObject *stored_self = to_this->__self__;\n"; + out << " if (stored_self == nullptr) {\n"; + out << " // Allocate a new Python instance\n"; + out << " Dtool_PyInstDef *self = (Dtool_PyInstDef *)PyType_GenericAlloc(&Dtool_" << ClassName << "._PyType, 0);\n"; + out << " self->_signature = PY_PANDA_SIGNATURE;\n"; + out << " self->_My_Type = &Dtool_" << ClassName << ";\n"; + out << " self->_ptr_to_object = to_this;\n"; + out << " self->_memory_rules = true;\n"; + out << " self->_is_const = false;\n"; + out << " to_this->__self__ = Py_NewRef((PyObject *)self);\n"; + out << " return Py_NewRef((PyObject *)self);\n"; + out << " }\n"; + out << " PyObject *result = Py_NewRef(stored_self);\n"; + out << " bool nonzero = to_this->unref();\n"; + out << " nassertr(nonzero, result);\n"; + out << " return result;\n"; + } else { + out << " // Allocate a new Python instance\n"; + out << " Dtool_PyInstDef *self = (Dtool_PyInstDef *)PyType_GenericAlloc(&Dtool_" << ClassName << "._PyType, 0);\n"; + out << " self->_signature = PY_PANDA_SIGNATURE;\n"; + out << " self->_My_Type = &Dtool_" << ClassName << ";\n"; + out << " self->_ptr_to_object = to_this;\n"; + out << " self->_memory_rules = false;\n"; + out << " self->_is_const = false;\n"; + out << " return (PyObject *)self;\n"; + } out << "}\n\n"; } } @@ -1678,6 +1845,9 @@ write_module_class(ostream &out, Object *obj) { std::string cClassName = obj->_itype.get_true_name(); std::string export_class_name = classNameFromCppName(obj->_itype.get_name(), false); + CPPStructType *struct_type = obj->_itype._cpptype->as_struct_type(); + bool py_subclassable = is_python_subclassable(struct_type); + out << "/**\n"; out << " * Python method tables for " << ClassName << " (" << export_class_name << ")\n" ; out << " */\n"; @@ -2558,22 +2728,36 @@ write_module_class(ostream &out, Object *obj) { // Find the remap. There should be only one. FunctionRemap *remap = *def._remaps.begin(); - const char *container = ""; - if (remap->_has_this) { - out << " " << cClassName << " *local_this = nullptr;\n"; - out << " DTOOL_Call_ExtractThisPointerForType(self, &Dtool_" << ClassName << ", (void **) &local_this);\n"; - out << " if (local_this == nullptr) {\n"; - out << " return 0;\n"; - out << " }\n\n"; - container = "local_this"; + out << " " << cClassName << " *local_this = nullptr;\n"; + out << " DTOOL_Call_ExtractThisPointerForType(self, &Dtool_" << ClassName << ", (void **)&local_this);\n"; + out << " if (local_this == nullptr) {\n"; + out << " return 0;\n"; + out << " }\n\n"; + + // Only participate in cycle detection if there are no C++ + // references to this object. + out << " if (local_this->get_ref_count() != (int)((Dtool_PyInstDef *)self)->_memory_rules) {\n"; + out << " return 0;\n"; + out << " }\n\n"; + + if (py_subclassable) { + // Python-subclassable types store a reference to their Python + // wrapper. + if (struct_type->is_final()) { + out << " if (Py_TYPE(self) != &Dtool_" << ClassName << "._PyType) {\n"; + } else { + out << " if (Py_TYPE(self) != &Dtool_" << ClassName << "._PyType && DtoolInstance_TYPE(self) == &Dtool_" << ClassName << ") {\n"; + } + out << " Py_VISIT(self);\n"; + out << " }\n"; } vector_string params((int)remap->_has_this); params.push_back("visit"); params.push_back("arg"); - out << " return " << remap->call_function(out, 2, false, container, params) << ";\n"; + out << " return " << remap->call_function(out, 2, false, "local_this", params) << ";\n"; out << "}\n\n"; } break; @@ -2861,6 +3045,56 @@ write_module_class(ostream &out, Object *obj) { out << "}\n\n"; } + bool has_gc = (slots.count("tp_traverse") || has_self_member(struct_type)); + bool has_local_traverse = false; + bool has_local_clear = false; + if (py_subclassable || has_gc) { + // Note that in the case of py_subclassable, the traverse function will + // only be called if the class is subclassed from Python, since we don't + // set Py_TPFLAGS_HAVE_GC, so we don't need to check whether it's a proxy. + if (!slots.count("tp_traverse")) { + has_local_traverse = true; + out << "static int Dtool_Traverse_" << ClassName << "(PyObject *self, visitproc visit, void *arg) {\n"; + out << " // If the only reference remaining is the one held by the Python wrapper,\n"; + out << " // report the circular reference to Python's GC, so that it can break it.\n"; + out << " " << cClassName << " *local_this = nullptr;\n"; + out << " if (!Dtool_Call_ExtractThisPointer(self, Dtool_" << ClassName << ", (void **)&local_this)) {\n"; + out << " return -1;\n"; + out << " }\n"; + out << " if (local_this->get_ref_count() == (int)((Dtool_PyInstDef *)self)->_memory_rules) {\n"; + if (py_subclassable) { + out << " Py_VISIT(self);\n"; + } + if (has_self_member(struct_type)) { + out << " Py_VISIT(local_this->__self__);\n"; + } + out << " }\n"; + out << " return 0;\n"; + out << "}\n"; + out << "\n"; + } + + if (!slots.count("tp_clear")) { + has_local_clear = true; + out << "static int Dtool_Clear_" << ClassName << "(PyObject *self) {\n"; + if (has_self_member(struct_type)) { + out << " Py_CLEAR(local_this->__self__);\n"; + } + if (py_subclassable) { + if (struct_type->is_final()) { + out << " if (Py_TYPE(self) != &Dtool_" << ClassName << "._PyType) {\n"; + } else { + out << " if (Py_TYPE(self) != &Dtool_" << ClassName << "._PyType && DtoolInstance_TYPE(self) == &Dtool_" << ClassName << ") {\n"; + } + out << " DtoolProxy_" << ClassName << " *proxy = (DtoolProxy_" << ClassName << " *)DtoolInstance_VOID_PTR(self);\n"; + out << " Py_CLEAR(proxy->DtoolProxy::_self);\n"; + out << " }\n"; + } + out << " return 0;\n"; + out << "}\n\n"; + } + } + int num_getset = 0; if (obj->_properties.size() > 0) { @@ -3151,7 +3385,7 @@ write_module_class(ostream &out, Object *obj) { } string gcflag; - if (obj->_protocol_types & Object::PT_python_gc) { + if (has_gc && slots.count("tp_traverse")) { gcflag = " | Py_TPFLAGS_HAVE_GC"; } @@ -3179,12 +3413,22 @@ write_module_class(ostream &out, Object *obj) { } // traverseproc tp_traverse; - out << " nullptr, // tp_traverse\n"; - //write_function_slot(out, 4, slots, "tp_traverse"); + if (has_local_traverse) { + out << " &Dtool_Traverse_" << ClassName << ",\n"; + } else if (has_gc) { + write_function_slot(out, 4, slots, "tp_traverse"); + } else { + out << " nullptr, // tp_traverse\n"; + } // inquiry tp_clear; - out << " nullptr, // tp_clear\n"; - //write_function_slot(out, 4, slots, "tp_clear"); + if (has_local_clear) { + out << " &Dtool_Clear_" << ClassName << ",\n"; + } else if (has_gc) { + write_function_slot(out, 4, slots, "tp_clear"); + } else { + out << " nullptr, // tp_clear\n"; + } // richcmpfunc tp_richcompare; if (has_local_richcompare) { @@ -3237,7 +3481,7 @@ write_module_class(ostream &out, Object *obj) { // newfunc tp_new; write_function_slot(out, 4, slots, "tp_new", "Dtool_new_" + ClassName); // freefunc tp_free; - if (obj->_protocol_types & Object::PT_python_gc) { + if (slots.count("tp_traverse") || has_self_member(struct_type)) { out << " PyObject_GC_Del,\n"; } else { out << " PyObject_Del,\n"; @@ -3277,9 +3521,9 @@ write_module_class(ostream &out, Object *obj) { out << " Dtool_UpcastInterface_" << ClassName << ",\n"; out << " Dtool_Wrap_" << ClassName << ",\n"; - int has_coerce = has_coerce_constructor(obj->_itype._cpptype->as_struct_type()); + int has_coerce = has_coerce_constructor(struct_type); if (has_coerce > 0) { - if (TypeManager::is_reference_count(obj->_itype._cpptype)) { + if (TypeManager::is_reference_count(struct_type)) { out << " (CoerceFunction)Dtool_ConstCoerce_" << ClassName << ",\n"; if (has_coerce > 1) { out << " (CoerceFunction)Dtool_Coerce_" << ClassName << ",\n"; @@ -6150,8 +6394,12 @@ write_function_instance(ostream &out, FunctionRemap *remap, indent_level += 2; } - if (is_constructor && !remap->_has_this && - (remap->_flags & FunctionRemap::F_explicit_self) != 0) { + bool init_self_member = is_constructor && + (return_flags & RF_coerced) == 0 && + has_self_member(remap->_cpptype->as_struct_type()); + + if (init_self_member || (is_constructor && !remap->_has_this && + (remap->_flags & FunctionRemap::F_explicit_self) != 0)) { // If we'll be passing "self" to the constructor, we need to pre- // initialize it here. Unfortunately, we can't pre-load the "this" // pointer, but the constructor itself can do this. @@ -6231,6 +6479,41 @@ write_function_instance(ostream &out, FunctionRemap *remap, } return_expr = "&coerced"; + } else if (is_constructor && (return_flags & RF_coerced) == 0 && + is_python_subclassable(remap->_cpptype->as_struct_type())) { + // Add special code to detect whether we're trying to create a subclass + // of this object. If so, create a proxy instance instead. + CPPType *orig_type = remap->_return_type->get_orig_type(); + TypeIndex type_index = builder.get_type(TypeManager::unwrap(TypeManager::resolve_type(orig_type)), false); + const InterrogateType &itype = idb->get_type(type_index); + std::string safe_name = make_safe_name(itype.get_scoped_name()); + + CPPType *type = remap->_return_type->get_temporary_type(); + type->output_instance(indent(out, indent_level), "return_value", &parser); + out << ";\n"; + + indent(out, indent_level) + << "if (Py_TYPE(self) != &Dtool_" << safe_name << "._PyType) {\n"; + indent(out, indent_level) + << " DtoolProxy_" << safe_name << " *proxy = new DtoolProxy_" << safe_name << "("; + remap->write_call_args(out, pexprs); + out << ");\n"; + indent(out, indent_level) + << " DtoolProxy_Init(proxy, self, Dtool_" << safe_name << ", &Dtool_WrapProxy_" << safe_name << ");\n"; + indent(out, indent_level) + << " return_value = proxy;\n"; + indent(out, indent_level) + << "} else {\n"; + + return_expr = remap->call_function(out, indent_level, true, container, pexprs); + indent(out, indent_level) + << " return_value = " << return_expr << ";\n"; + indent(out, indent_level) + << "}\n"; + + return_expr = "return_value"; + manage_return = true; + } else { // The general case; an ordinary constructor or function. return_expr = remap->call_function(out, indent_level, true, container, pexprs); @@ -6290,6 +6573,11 @@ write_function_instance(ostream &out, FunctionRemap *remap, indent(out, indent_level) << "}\n"; } + if (init_self_member) { + indent(out, indent_level) + << "return_value->__self__ = Py_NewRef(self);\n"; + } + if (TypeManager::is_pointer_to_PyObject(remap->_return_type->get_orig_type())) { return_expr = "Py_XNewRef(return_value)"; } else { @@ -8054,6 +8342,53 @@ IsRunTimeTyped(const InterrogateType &itype) { return false; } +/** + * Returns true if this object has special support for inheriting from Python. + */ +bool InterfaceMakerPythonNative:: +is_python_subclassable(CPPStructType *struct_type) { + if (struct_type != nullptr && !struct_type->is_final() && + IsPandaTypedObject(struct_type) && + TypeManager::is_reference_count(struct_type)) { + + // Make sure the type has a published constructor. + TypeIndex type_index = builder.get_type(struct_type, false); + if (type_index != 0) { + Objects::iterator oi = _objects.find(type_index); + if (oi != _objects.end()) { + Object *obj = (*oi).second; + return !obj->_constructors.empty(); + } + } + } + return false; +} + +/** + * Returns true if the type has a public member named __self__. + */ +bool InterfaceMakerPythonNative:: +has_self_member(CPPStructType *struct_type) { + if (struct_type == nullptr) { + return false; + } + + CPPScope *scope = struct_type->get_scope(); + if (scope != nullptr && scope->_variables.count("__self__")) { + return true; + } + + for (const CPPStructType::Base &base : struct_type->_derivation) { + if (base._base != nullptr) { + CPPStructType *base_type = base._base->as_struct_type(); + if (base_type != nullptr && has_self_member(base_type)) { + return true; + } + } + } + return false; +} + /** */ diff --git a/dtool/src/interrogate/interfaceMakerPythonNative.h b/dtool/src/interrogate/interfaceMakerPythonNative.h index f03a063d64..aec04a6652 100644 --- a/dtool/src/interrogate/interfaceMakerPythonNative.h +++ b/dtool/src/interrogate/interfaceMakerPythonNative.h @@ -200,6 +200,8 @@ public: bool isExportThisRun(Function *func); bool isFunctionWithThis( Function *func); bool IsRunTimeTyped(const InterrogateType &itype); + bool is_python_subclassable(CPPStructType *type); + bool has_self_member(CPPStructType *type); // comunicates the cast capabilites among methods.. struct CastDetails { diff --git a/dtool/src/interrogatedb/py_compat.h b/dtool/src/interrogatedb/py_compat.h index 0344bcd149..4b67fba763 100644 --- a/dtool/src/interrogatedb/py_compat.h +++ b/dtool/src/interrogatedb/py_compat.h @@ -239,6 +239,10 @@ INLINE PyObject *PyObject_CallMethodNoArgs(PyObject *obj, PyObject *name) { INLINE PyObject *PyObject_CallMethodOneArg(PyObject *obj, PyObject *name, PyObject *arg) { return PyObject_CallMethodObjArgs(obj, name, arg, nullptr); } + +INLINE int PyObject_GC_IsTracked(PyObject *obj) { + return _PyObject_GC_IS_TRACKED(obj); +} #endif /* Python 3.10 */ diff --git a/dtool/src/interrogatedb/py_panda.cxx b/dtool/src/interrogatedb/py_panda.cxx index 888a25a7ae..b691fbef48 100644 --- a/dtool/src/interrogatedb/py_panda.cxx +++ b/dtool/src/interrogatedb/py_panda.cxx @@ -540,6 +540,32 @@ Dtool_TypeMap *Dtool_GetGlobalTypeMap() { } } +/** + * + */ +void DtoolProxy_Init(DtoolProxy *proxy, PyObject *self, + Dtool_PyTypedObject &classdef, + TypeRegistry::PythonWrapFunc *wrap_func) { + if (proxy == nullptr) { + // Out of memory, the generated code will handle this. + return; + } + + proxy->_self = Py_NewRef(self); + PyTypeObject *cls = Py_TYPE(self); + if (cls != &classdef._PyType) { + TypeRegistry *registry = TypeRegistry::ptr(); + TypeHandle handle = registry->register_dynamic_type(cls->tp_name); + registry->record_derivation(handle, classdef._type); + //TODO unregister type when it is unloaded? weak callback? + PyTypeObject *cls_ref = (PyTypeObject *)Py_NewRef((PyObject *)cls); + registry->record_python_type(handle, cls_ref, wrap_func); + proxy->_type = handle; + } else { + proxy->_type = classdef._type; + } +} + #define PY_MAJOR_VERSION_STR #PY_MAJOR_VERSION "." #PY_MINOR_VERSION #if PY_MAJOR_VERSION >= 3 diff --git a/dtool/src/interrogatedb/py_panda.h b/dtool/src/interrogatedb/py_panda.h index 1acfd17d74..9d8fc113ff 100644 --- a/dtool/src/interrogatedb/py_panda.h +++ b/dtool/src/interrogatedb/py_panda.h @@ -182,6 +182,16 @@ typedef std::map Dtool_TypeMap; EXPCL_PYPANDA Dtool_TypeMap *Dtool_GetGlobalTypeMap(); +class DtoolProxy { +public: + mutable PyObject *_self; + TypeHandle _type; +}; + +EXPCL_PYPANDA void DtoolProxy_Init(DtoolProxy *proxy, PyObject *self, + Dtool_PyTypedObject &classdef, + TypeRegistry::PythonWrapFunc *wrap_func); + /** */ diff --git a/panda/src/putil/typedWritable_ext.cxx b/panda/src/putil/typedWritable_ext.cxx index a61bcdfac4..699794b368 100644 --- a/panda/src/putil/typedWritable_ext.cxx +++ b/panda/src/putil/typedWritable_ext.cxx @@ -31,7 +31,7 @@ extern Dtool_PyTypedObject Dtool_TypeHandle; /** * Class that upcalls to the parent class when write_datagram is called. */ -class TypedWritableProxy : public TypedWritable { +class TypedWritableProxy : public TypedWritable, public DtoolProxy { public: ~TypedWritableProxy() { } @@ -123,10 +123,6 @@ public: virtual TypeHandle force_init_type() override { return _type; } - -public: - PyObject *_self; - TypeHandle _type; }; /** diff --git a/panda/src/recorder/mouseRecorder.h b/panda/src/recorder/mouseRecorder.h index a30c2d428a..1c29a42152 100644 --- a/panda/src/recorder/mouseRecorder.h +++ b/panda/src/recorder/mouseRecorder.h @@ -79,7 +79,7 @@ public: INLINE virtual int get_ref_count() const final { return ReferenceCount::get_ref_count(); }; INLINE virtual void ref() const final { ReferenceCount::ref(); }; - INLINE virtual bool unref() const final { return ReferenceCount::unref(); }; + INLINE virtual bool unref() const { return ReferenceCount::unref(); }; protected: static TypedWritable *make_from_bam(const FactoryParams ¶ms); diff --git a/tests/event/test_event.py b/tests/event/test_event.py new file mode 100644 index 0000000000..947ed093a3 --- /dev/null +++ b/tests/event/test_event.py @@ -0,0 +1,64 @@ +from panda3d.core import Event, EventQueue +from contextlib import contextmanager +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_event_subclass_gc(): + class PythonEvent(Event): + pass + + # Only subclasses should be tracked by the GC + assert not gc.is_tracked(Event('test')) + + # Python wrapper destructs last + with gc_disabled(): + event = PythonEvent('test_event_subclass_gc1') + + assert len(gc.get_objects()) == 1 + assert gc.get_objects()[0] == event + + event = None + + gc.collect() + assert len(gc.garbage) == 1 + assert gc.garbage[0].name == 'test_event_subclass_gc1' + + # C++ reference destructs last + with gc_disabled(): + event = PythonEvent('test_event_subclass_gc2') + queue = EventQueue() + queue.queue_event(event) + + assert event in gc.get_objects() + + event = None + + # Hasn't been collected yet, since parent still holds a reference + gc.collect() + assert len(gc.garbage) == 0 + assert 'test_event_subclass_gc2' in [obj.name for obj in gc.get_objects() if isinstance(obj, Event)] + + queue = None + + # Destructed without needing the GC + assert 'test_event_subclass_gc2' not in [obj.name for obj in gc.get_objects() if isinstance(obj, Event)] + + gc.collect() + assert len(gc.garbage) == 0 diff --git a/tests/pgraph/test_pandanode.py b/tests/pgraph/test_pandanode.py index 28e684c04b..9870b4d3c6 100644 --- a/tests/pgraph/test_pandanode.py +++ b/tests/pgraph/test_pandanode.py @@ -1,4 +1,25 @@ from panda3d.core import PandaNode, TransformState +from contextlib import contextmanager +import pytest +import gc + + +@contextmanager +def gc_disabled(): + gc.disable() + gc.collect() + gc.freeze() + gc.set_debug(gc.DEBUG_SAVEALL) + gc.garbage.clear() + + try: + yield + finally: + gc.set_debug(0) + gc.garbage.clear() + gc.unfreeze() + gc.collect() + gc.enable() def test_node_prev_transform(): @@ -34,3 +55,123 @@ def test_node_prev_transform(): assert node.transform == t3 assert node.prev_transform == t3 assert not node.has_dirty_prev_transform() + + +def test_node_tag_cycle(): + with gc_disabled(): + node = PandaNode('test_node_tag_cycle') + node.set_python_tag('self', node) + node.set_python_tag('self2', node) + assert gc.is_tracked(node) + node = None + + gc.collect() + assert len(gc.garbage) > 0 + + for g in gc.garbage: + if isinstance(g, PandaNode) and g.name == 'test_node_tag_cycle': + break + else: + assert False + + +@pytest.mark.xfail +def test_node_tag_cycle_multiple_wrappers(): + # Doesn't yet work since the traverse checks that the refcount is 1. + with gc_disabled(): + node = PandaNode('test_node_tag_cycle_multiple_wrappers') + node.set_python_tag('self', node) + + # Find another reference to the same node, we do this by temporarily + # attaching a child. + child = PandaNode('child') + node.add_child(child) + node2 = child.get_parent(0) + node.remove_child(0) + child = None + + assert node2 == node + assert node2 is not node + assert node2.this == node.this + + node.set_python_tag('self2', node2) + + node = None + node2 = None + + gc.collect() + assert len(gc.garbage) > 0 + + for g in gc.garbage: + if isinstance(g, PandaNode) and g.name == 'test_node_tag_cycle_multiple_wrappers': + break + else: + pytest.fail('not found in garbage') + + +def test_node_subclass_persistent(): + class NodeSubclass(PandaNode): + pass + + node = NodeSubclass('test_node_subclass') + assert isinstance(node, PandaNode) + + # Always GC-tracked + assert gc.is_tracked(node) + + # Registered type handle + type_handle = node.get_type() + assert type_handle.name == 'NodeSubclass' + assert type_handle != PandaNode.get_class_type() + assert tuple(type_handle.parent_classes) == (PandaNode.get_class_type(), ) + + # Persistent wrapper + parent = PandaNode('parent') + parent.add_child(node) + child = parent.get_child(0) + assert child.this == node.this + assert child is node + assert type(child) is NodeSubclass + parent = None + child = None + + +def test_node_subclass_gc(): + class NodeSubclass(PandaNode): + pass + + # Python wrapper destructs last + with gc_disabled(): + node = NodeSubclass('test_node_subclass_gc1') + + assert len(gc.get_objects()) == 1 + assert gc.get_objects()[0] == node + + node = None + + gc.collect() + assert len(gc.garbage) == 1 + assert gc.garbage[0].name == 'test_node_subclass_gc1' + + # C++ reference destructs last + with gc_disabled(): + node = NodeSubclass('test_node_subclass_gc2') + parent = PandaNode('parent') + parent.add_child(node) + + assert node in gc.get_objects() + + node = None + + # Hasn't been collected yet, since parent still holds a reference + gc.collect() + assert len(gc.garbage) == 0 + assert 'test_node_subclass_gc2' in [obj.name for obj in gc.get_objects() if isinstance(obj, PandaNode)] + + parent = None + + # Destructed without needing the GC + assert 'test_node_subclass_gc2' not in [obj.name for obj in gc.get_objects() if isinstance(obj, PandaNode)] + + gc.collect() + assert len(gc.garbage) == 0