diff options
author | Mark Spruiell <mes@zeroc.com> | 2017-12-27 05:39:15 -0800 |
---|---|---|
committer | Mark Spruiell <mes@zeroc.com> | 2017-12-27 05:39:15 -0800 |
commit | fc1bb7f6d9d6d7ba03ab332c561d38a441cf34b2 (patch) | |
tree | 59774a35f6117cc5e6f39a8db21ac91fb75f474b | |
parent | IceSSL compatibility fixes for OpenSSL 1.1 (diff) | |
download | ice-fc1bb7f6d9d6d7ba03ab332c561d38a441cf34b2.tar.bz2 ice-fc1bb7f6d9d6d7ba03ab332c561d38a441cf34b2.tar.xz ice-fc1bb7f6d9d6d7ba03ab332c561d38a441cf34b2.zip |
ICE-8496 - Python crash on exit
-rw-r--r-- | py/modules/IcePy/ObjectFactory.cpp | 2 | ||||
-rw-r--r-- | py/modules/IcePy/Operation.cpp | 4 | ||||
-rw-r--r-- | py/modules/IcePy/Types.cpp | 182 | ||||
-rw-r--r-- | py/modules/IcePy/Types.h | 22 |
4 files changed, 82 insertions, 128 deletions
diff --git a/py/modules/IcePy/ObjectFactory.cpp b/py/modules/IcePy/ObjectFactory.cpp index 7bf97bb9eb4..7fe72706ab6 100644 --- a/py/modules/IcePy/ObjectFactory.cpp +++ b/py/modules/IcePy/ObjectFactory.cpp @@ -97,7 +97,7 @@ IcePy::ObjectFactory::create(const string& id) // // Instantiate the object. // - PyTypeObject* type = reinterpret_cast<PyTypeObject*>(info->pythonType.get()); + PyTypeObject* type = reinterpret_cast<PyTypeObject*>(info->pythonType); PyObjectHandle args = PyTuple_New(0); PyObjectHandle obj = type->tp_new(type, args.get(), 0); if(!obj.get()) diff --git a/py/modules/IcePy/Operation.cpp b/py/modules/IcePy/Operation.cpp index c941de47516..9200b18af80 100644 --- a/py/modules/IcePy/Operation.cpp +++ b/py/modules/IcePy/Operation.cpp @@ -1738,7 +1738,7 @@ IcePy::TypedInvocation::validateException(PyObject* ex) const { for(ExceptionInfoList::const_iterator p = _op->exceptions.begin(); p != _op->exceptions.end(); ++p) { - if(PyObject_IsInstance(ex, (*p)->pythonType.get())) + if(PyObject_IsInstance(ex, (*p)->pythonType)) { return true; } @@ -3581,7 +3581,7 @@ IcePy::TypedUpcall::validateException(PyObject* ex) const { for(ExceptionInfoList::const_iterator p = _op->exceptions.begin(); p != _op->exceptions.end(); ++p) { - if(PyObject_IsInstance(ex, (*p)->pythonType.get())) + if(PyObject_IsInstance(ex, (*p)->pythonType)) { return true; } diff --git a/py/modules/IcePy/Types.cpp b/py/modules/IcePy/Types.cpp index 347e5f7907d..f45a866d54a 100644 --- a/py/modules/IcePy/Types.cpp +++ b/py/modules/IcePy/Types.cpp @@ -42,14 +42,6 @@ static ExceptionInfoMap _exceptionInfoMap; namespace IcePy { -class InfoMapDestroyer -{ -public: - - ~InfoMapDestroyer(); -}; -static InfoMapDestroyer infoMapDestroyer; - class ReadObjectCallback : public Ice::ReadObjectCallback { public: @@ -139,6 +131,12 @@ extern "C" static void typeInfoDealloc(TypeInfoObject* self) { + // + // Every TypeInfo object is assigned to a "_t_XXX" variable in its enclosing module. Python releases these + // objects during shutdown, which gives us a chance to release resources and break cyclic references. + // + assert(self->info); + (*self->info)->destroy(); delete self->info; Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self)); } @@ -940,8 +938,6 @@ IcePy::EnumInfo::EnumInfo(const string& ident, PyObject* t, PyObject* e) : assert(PyType_Check(t)); assert(PyDict_Check(e)); - Py_INCREF(t); - Py_ssize_t pos = 0; PyObject* key; PyObject* value; @@ -975,7 +971,7 @@ IcePy::EnumInfo::getId() const bool IcePy::EnumInfo::validate(PyObject* val) { - return PyObject_IsInstance(val, pythonType.get()) == 1; + return PyObject_IsInstance(val, pythonType) == 1; } bool @@ -999,7 +995,7 @@ IcePy::EnumInfo::optionalFormat() const void IcePy::EnumInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap*, bool optional, const Ice::StringSeq*) { - assert(PyObject_IsInstance(p, pythonType.get()) == 1); // validate() should have caught this. + assert(PyObject_IsInstance(p, pythonType) == 1); // validate() should have caught this. // // Validate value. @@ -1066,6 +1062,12 @@ IcePy::EnumInfo::print(PyObject* value, IceUtilInternal::Output& out, PrintObjec out << getString(p.get()); } +void +IcePy::EnumInfo::destroy() +{ + const_cast<EnumeratorMap&>(enumerators).clear(); +} + // // DataMember implementation. // @@ -1089,7 +1091,7 @@ convertDataMembers(PyObject* members, DataMemberList& reqMembers, DataMemberList { PyObject* m = PyTuple_GET_ITEM(members, i); assert(PyTuple_Check(m)); - assert(PyTuple_GET_SIZE(m) == allowOptional ? 5 : 3); + assert(PyTuple_GET_SIZE(m) == (allowOptional ? 5 : 3)); PyObject* name = PyTuple_GET_ITEM(m, 0); // Member name. assert(checkString(name)); @@ -1164,8 +1166,6 @@ IcePy::StructInfo::StructInfo(const string& ident, PyObject* t, PyObject* m) : assert(PyType_Check(t)); assert(PyTuple_Check(m)); - Py_INCREF(t); - DataMemberList opt; convertDataMembers(m, const_cast<DataMemberList&>(members), opt, false); assert(opt.empty()); @@ -1191,7 +1191,7 @@ IcePy::StructInfo::getId() const bool IcePy::StructInfo::validate(PyObject* val) { - return PyObject_IsInstance(val, pythonType.get()) == 1; + return PyObject_IsInstance(val, pythonType) == 1; } bool @@ -1230,7 +1230,7 @@ void IcePy::StructInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap* objectMap, bool optional, const Ice::StringSeq*) { - assert(PyObject_IsInstance(p, pythonType.get()) == 1); // validate() should have caught this. + assert(PyObject_IsInstance(p, pythonType) == 1); // validate() should have caught this. int sizePos = -1; if(optional) @@ -1281,7 +1281,7 @@ IcePy::StructInfo::unmarshal(const Ice::InputStreamPtr& is, const UnmarshalCallb void* closure, bool optional, const Ice::StringSeq*) { PyObjectHandle args = PyTuple_New(0); - PyTypeObject* type = reinterpret_cast<PyTypeObject*>(pythonType.get()); + PyTypeObject* type = reinterpret_cast<PyTypeObject*>(pythonType); PyObjectHandle p = type->tp_new(type, args.get(), 0); if(!p.get()) { @@ -1340,10 +1340,6 @@ IcePy::StructInfo::print(PyObject* value, IceUtilInternal::Output& out, PrintObj void IcePy::StructInfo::destroy() { - for(DataMemberList::const_iterator p = members.begin(); p != members.end(); ++p) - { - (*p)->type->destroy(); - } const_cast<DataMemberList&>(members).clear(); } @@ -1611,11 +1607,7 @@ IcePy::SequenceInfo::print(PyObject* value, IceUtilInternal::Output& out, PrintO void IcePy::SequenceInfo::destroy() { - if(elementType) - { - elementType->destroy(); - const_cast<TypeInfoPtr&>(elementType) = 0; - } + const_cast<TypeInfoPtr&>(elementType) = 0; } PyObject* @@ -2281,7 +2273,7 @@ IcePy::CustomInfo::getId() const bool IcePy::CustomInfo::validate(PyObject* val) { - return PyObject_IsInstance(val, pythonType.get()) == 1; + return PyObject_IsInstance(val, pythonType) == 1; } bool @@ -2312,7 +2304,7 @@ void IcePy::CustomInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap* objectMap, bool, const Ice::StringSeq* metaData) { - assert(PyObject_IsInstance(p, pythonType.get()) == 1); // validate() should have caught this. + assert(PyObject_IsInstance(p, pythonType) == 1); // validate() should have caught this. PyObjectHandle obj = PyObject_CallMethod(p, STRCAST("IsInitialized"), 0); if(!obj.get()) @@ -2363,7 +2355,7 @@ IcePy::CustomInfo::unmarshal(const Ice::InputStreamPtr& is, const UnmarshalCallb assert(PyErr_Occurred()); throw AbortMarshaling(); } - PyTypeObject* type = reinterpret_cast<PyTypeObject*>(pythonType.get()); + PyTypeObject* type = reinterpret_cast<PyTypeObject*>(pythonType); PyObjectHandle p = type->tp_new(type, args.get(), 0); if(!p.get()) { @@ -2426,11 +2418,6 @@ IcePy::CustomInfo::print(PyObject* value, IceUtilInternal::Output& out, PrintObj } } -void -IcePy::CustomInfo::destroy() -{ -} - // // DictionaryInfo implementation. // @@ -2663,16 +2650,8 @@ IcePy::DictionaryInfo::KeyCallback::unmarshaled(PyObject* val, PyObject*, void*) void IcePy::DictionaryInfo::destroy() { - if(keyType) - { - keyType->destroy(); - keyType = 0; - } - if(valueType) - { - valueType->destroy(); - valueType = 0; - } + keyType = 0; + valueType = 0; } // @@ -2681,7 +2660,7 @@ IcePy::DictionaryInfo::destroy() IcePy::ClassInfo::ClassInfo(const string& ident) : id(ident), compactId(-1), isAbstract(false), preserve(false), defined(false) { - const_cast<PyObjectHandle&>(typeObj) = createType(this); + typeObj = createType(this); } void @@ -2713,8 +2692,7 @@ IcePy::ClassInfo::define(PyObject* t, int compact, bool abstr, bool pres, PyObje convertDataMembers(m, const_cast<DataMemberList&>(members), const_cast<DataMemberList&>(optionalMembers), true); - const_cast<PyObjectHandle&>(pythonType) = t; - Py_INCREF(t); + pythonType = t; const_cast<bool&>(defined) = true; } @@ -2728,7 +2706,7 @@ IcePy::ClassInfo::getId() const bool IcePy::ClassInfo::validate(PyObject* val) { - return val == Py_None || PyObject_IsInstance(val, pythonType.get()) == 1; + return val == Py_None || PyObject_IsInstance(val, pythonType) == 1; } bool @@ -2759,7 +2737,7 @@ void IcePy::ClassInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap* objectMap, bool, const Ice::StringSeq*) { - if(!pythonType.get()) + if(!pythonType) { PyErr_Format(PyExc_RuntimeError, STRCAST("class %s is declared but not defined"), id.c_str()); throw AbortMarshaling(); @@ -2771,7 +2749,7 @@ IcePy::ClassInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap return; } - if(!PyObject_IsInstance(p, pythonType.get())) + if(!PyObject_IsInstance(p, pythonType)) { PyErr_Format(PyExc_ValueError, STRCAST("expected value of type %s"), id.c_str()); throw AbortMarshaling(); @@ -2806,7 +2784,7 @@ void IcePy::ClassInfo::unmarshal(const Ice::InputStreamPtr& is, const UnmarshalCallbackPtr& cb, PyObject* target, void* closure, bool, const Ice::StringSeq*) { - if(!pythonType.get()) + if(!pythonType) { PyErr_Format(PyExc_RuntimeError, STRCAST("class %s is declared but not defined"), id.c_str()); throw AbortMarshaling(); @@ -2868,16 +2846,7 @@ IcePy::ClassInfo::destroy() { const_cast<ClassInfoPtr&>(base) = 0; const_cast<ClassInfoList&>(interfaces).clear(); - if(!members.empty()) - { - DataMemberList ml = members; - const_cast<DataMemberList&>(members).clear(); - for(DataMemberList::iterator p = ml.begin(); p != ml.end(); ++p) - { - (*p)->type->destroy(); - } - } - const_cast<PyObjectHandle&>(typeObj) = 0; // Break circular reference. + const_cast<DataMemberList&>(members).clear(); } void @@ -2933,14 +2902,13 @@ IcePy::ClassInfo::printMembers(PyObject* value, IceUtilInternal::Output& out, Pr IcePy::ProxyInfo::ProxyInfo(const string& ident) : id(ident) { - const_cast<PyObjectHandle&>(typeObj) = createType(this); + typeObj = createType(this); // Borrowed reference. } void IcePy::ProxyInfo::define(PyObject* t) { - const_cast<PyObjectHandle&>(pythonType) = t; - Py_INCREF(t); + pythonType = t; // Borrowed reference. } string @@ -2952,7 +2920,7 @@ IcePy::ProxyInfo::getId() const bool IcePy::ProxyInfo::validate(PyObject* val) { - return val == Py_None || PyObject_IsInstance(val, pythonType.get()) == 1; + return val == Py_None || PyObject_IsInstance(val, pythonType) == 1; } bool @@ -3024,13 +2992,13 @@ IcePy::ProxyInfo::unmarshal(const Ice::InputStreamPtr& is, const UnmarshalCallba return; } - if(!pythonType.get()) + if(!pythonType) { PyErr_Format(PyExc_RuntimeError, STRCAST("class %s is declared but not defined"), id.c_str()); throw AbortMarshaling(); } - PyObjectHandle p = createProxy(proxy, is->communicator(), pythonType.get()); + PyObjectHandle p = createProxy(proxy, is->communicator(), pythonType); cb->unmarshaled(p.get(), target, closure); } @@ -3059,12 +3027,6 @@ IcePy::ProxyInfo::print(PyObject* value, IceUtilInternal::Output& out, PrintObje } } -void -IcePy::ProxyInfo::destroy() -{ - const_cast<PyObjectHandle&>(typeObj) = 0; // Break circular reference. -} - // // ObjectWriter implementation. // @@ -3297,27 +3259,6 @@ IcePy::ObjectReader::getSlicedData() const } // -// InfoMapDestroyer implementation. -// -IcePy::InfoMapDestroyer::~InfoMapDestroyer() -{ - { - for(ProxyInfoMap::iterator p = _proxyInfoMap.begin(); p != _proxyInfoMap.end(); ++p) - { - p->second->destroy(); - } - } - { - for(ClassInfoMap::iterator p = _classInfoMap.begin(); p != _classInfoMap.end(); ++p) - { - p->second->destroy(); - } - } - _compactIdMap.clear(); - _exceptionInfoMap.clear(); -} - -// // ReadObjectCallback implementation. // IcePy::ReadObjectCallback::ReadObjectCallback(const ClassInfoPtr& info, const UnmarshalCallbackPtr& cb, @@ -3344,7 +3285,7 @@ IcePy::ReadObjectCallback::invoke(const Ice::ObjectPtr& p) // Verify that the object's type is compatible with the formal type. // PyObject* obj = reader->getObject(); // Borrowed reference. - if(!PyObject_IsInstance(obj, _info->pythonType.get())) + if(!PyObject_IsInstance(obj, _info->pythonType)) { Ice::UnexpectedObjectException ex(__FILE__, __LINE__); ex.reason = "unmarshaled object is not an instance of " + _info->id; @@ -3367,7 +3308,7 @@ IcePy::ReadObjectCallback::invoke(const Ice::ObjectPtr& p) void IcePy::ExceptionInfo::marshal(PyObject* p, const Ice::OutputStreamPtr& os, ObjectMap* objectMap) { - if(!PyObject_IsInstance(p, pythonType.get())) + if(!PyObject_IsInstance(p, pythonType)) { PyErr_Format(PyExc_ValueError, STRCAST("expected exception %s"), id.c_str()); throw AbortMarshaling(); @@ -3446,7 +3387,7 @@ IcePy::ExceptionInfo::writeMembers(PyObject* p, const Ice::OutputStreamPtr& os, PyObject* IcePy::ExceptionInfo::unmarshal(const Ice::InputStreamPtr& is) { - PyObjectHandle p = createExceptionInstance(pythonType.get()); + PyObjectHandle p = createExceptionInstance(pythonType); ExceptionInfoPtr info = this; while(info) @@ -3489,7 +3430,7 @@ IcePy::ExceptionInfo::unmarshal(const Ice::InputStreamPtr& is) void IcePy::ExceptionInfo::print(PyObject* value, IceUtilInternal::Output& out) { - if(!PyObject_IsInstance(value, pythonType.get())) + if(!PyObject_IsInstance(value, pythonType)) { out << "<invalid value - expected " << id << ">"; return; @@ -4063,10 +4004,13 @@ IcePy_declareProxy(PyObject*, PyObject* args) { info = new ProxyInfo(proxyId); addProxyInfo(proxyId, info); + return info->typeObj; // Delegate ownership to the global "_t_XXX" variable. + } + else + { + Py_INCREF(info->typeObj); + return info->typeObj; } - - Py_INCREF(info->typeObj.get()); - return info->typeObj.get(); } extern "C" @@ -4090,12 +4034,15 @@ IcePy_defineProxy(PyObject*, PyObject* args) { info = new ProxyInfo(proxyId); addProxyInfo(proxyId, info); + info->define(type); + return info->typeObj; // Delegate ownership to the global "_t_XXX" variable. + } + else + { + info->define(type); + Py_INCREF(info->typeObj); + return info->typeObj; } - - info->define(type); - - Py_INCREF(info->typeObj.get()); - return info->typeObj.get(); } extern "C" @@ -4113,10 +4060,13 @@ IcePy_declareClass(PyObject*, PyObject* args) { info = new ClassInfo(id); addClassInfo(id, info); + return info->typeObj; // Delegate ownership to the global "_t_XXX" variable. + } + else + { + Py_INCREF(info->typeObj); + return info->typeObj; } - - Py_INCREF(info->typeObj.get()); - return info->typeObj.get(); } extern "C" @@ -4140,6 +4090,8 @@ IcePy_defineClass(PyObject*, PyObject* args) assert(PyTuple_Check(meta)); + PyObject* r; + // // A ClassInfo object will already exist for this id if a forward declaration // was encountered, or if the Slice definition is being reloaded. In the latter @@ -4150,6 +4102,12 @@ IcePy_defineClass(PyObject*, PyObject* args) { info = new ClassInfo(id); addClassInfo(id, info); + r = info->typeObj; // Delegate ownership to the global "_t_XXX" variable. + } + else + { + Py_INCREF(info->typeObj); + r = info->typeObj; } info->define(type, compactId, isAbstract ? true : false, preserve ? true : false, base, interfaces, members); @@ -4161,8 +4119,7 @@ IcePy_defineClass(PyObject*, PyObject* args) } _compactIdMap.insert(CompactIdMap::value_type(info->compactId, info)); - Py_INCREF(info->typeObj.get()); - return info->typeObj.get(); + return r; } extern "C" @@ -4211,7 +4168,6 @@ IcePy_defineException(PyObject*, PyObject* args) } info->pythonType = type; - Py_INCREF(type); addExceptionInfo(id, info); diff --git a/py/modules/IcePy/Types.h b/py/modules/IcePy/Types.h index 81d77f7a512..15d9b4efd72 100644 --- a/py/modules/IcePy/Types.h +++ b/py/modules/IcePy/Types.h @@ -195,8 +195,10 @@ public: virtual void print(PyObject*, IceUtilInternal::Output&, PrintObjectHistory*); + virtual void destroy(); + const std::string id; - const PyObjectHandle pythonType; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. const Ice::Int maxValue; const EnumeratorMap enumerators; }; @@ -246,7 +248,7 @@ public: const std::string id; const DataMemberList members; - const PyObjectHandle pythonType; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. private: @@ -340,10 +342,8 @@ public: virtual void print(PyObject*, IceUtilInternal::Output&, PrintObjectHistory*); - virtual void destroy(); - const std::string id; - const PyObjectHandle pythonType; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. }; typedef IceUtil::Handle<CustomInfo> CustomInfoPtr; @@ -434,8 +434,8 @@ public: const ClassInfoList interfaces; const DataMemberList members; const DataMemberList optionalMembers; - const PyObjectHandle pythonType; - const PyObjectHandle typeObj; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. + PyObject* typeObj; // Borrowed reference - the "_t_XXX" variable owns the reference. const bool defined; }; @@ -464,11 +464,9 @@ public: virtual void print(PyObject*, IceUtilInternal::Output&, PrintObjectHistory*); - virtual void destroy(); - const std::string id; - const PyObjectHandle pythonType; - const PyObjectHandle typeObj; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. + PyObject* typeObj; // Borrowed reference - the "_t_XXX" variable owns the reference. }; typedef IceUtil::Handle<ProxyInfo> ProxyInfoPtr; @@ -491,7 +489,7 @@ public: DataMemberList members; DataMemberList optionalMembers; bool usesClasses; - PyObjectHandle pythonType; + PyObject* pythonType; // Borrowed reference - the enclosing Python module owns the reference. private: |