From e35160d035d33e84f83a762bce09289aa50b2006 Mon Sep 17 00:00:00 2001
From: Marco Biasini <marco.biasini@unibas.ch>
Date: Mon, 29 Jul 2013 14:42:24 +0200
Subject: [PATCH] make sure singletons don't reference any Python objects
The order of destruction of static objects, e.g. singletons is
arbitrary and can not be relied upon. For some versions of
Python, the singletons are destroyed after Python has shutdown.
When objects stored in the singletons have been created in Python,
a segfault is produced when they are destroyed, since the destructor
assumes a Python interpreter exists. To work around that, we register
atexit handlers to remove all references to Python objects. That's
required for the IOProfileRegistry and Conopology singletons.
---
modules/conop/pymod/export_conop.cc | 16 +++++++++++++++-
modules/io/pymod/__init__.py | 2 +-
modules/io/pymod/export_pdb_io.cc | 19 +++++++++++++++++--
modules/io/src/mol/io_profile.hh | 4 +++-
modules/io/tests/test_io_pdb.py | 8 ++++++++
5 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/modules/conop/pymod/export_conop.cc b/modules/conop/pymod/export_conop.cc
index 39f006b45..9f72861bd 100644
--- a/modules/conop/pymod/export_conop.cc
+++ b/modules/conop/pymod/export_conop.cc
@@ -25,12 +25,26 @@ using namespace boost::python;
using namespace ost::conop;
+void destroy_compound_lib() {
+ Conopology::Instance().SetDefaultLib(CompoundLibPtr());
+}
+
void export_Conop() {
class_<Conopology, boost::noncopyable>("Conopology", no_init)
- .def("Instance", &Conopology::Instance, return_value_policy<reference_existing_object>()).staticmethod("Instance")
+ .def("Instance", &Conopology::Instance,
+ return_value_policy<reference_existing_object>()).staticmethod("Instance")
.def("SetDefaultLib", &Conopology::SetDefaultLib)
.def("GetDefaultLib", &Conopology::GetDefaultLib)
;
+ // we need to make sure there are no pending references to Python objects
+ // tied to the Conopology singleton instance. The destructor of the
+ // Conopology may be called after Python is shutdown which results in a
+ // segfault.
+ scope().attr("__dict__")["atexit"]=handle<>(PyImport_ImportModule("atexit"));
+
+ def("_destroy_compound_lib", &destroy_compound_lib);
+ object r=scope().attr("_destroy_compound_lib");
+ scope().attr("atexit").attr("register")(r);
}
diff --git a/modules/io/pymod/__init__.py b/modules/io/pymod/__init__.py
index 584f03eff..85751e697 100644
--- a/modules/io/pymod/__init__.py
+++ b/modules/io/pymod/__init__.py
@@ -32,7 +32,7 @@ class IOProfiles:
def __setitem__(self, key, value):
if isinstance(value, str):
- value=self[value]
+ value=self[value].Copy()
IOProfileRegistry.Instance().Set(key, value)
self._dict[key]=value
diff --git a/modules/io/pymod/export_pdb_io.cc b/modules/io/pymod/export_pdb_io.cc
index c5062e20d..6fe425aa7 100644
--- a/modules/io/pymod/export_pdb_io.cc
+++ b/modules/io/pymod/export_pdb_io.cc
@@ -33,6 +33,10 @@ BOOST_PYTHON_FUNCTION_OVERLOADS(load_PDB_ov, LoadPDB, 1, 2)
void (PDBWriter::*write_a)(const mol::EntityHandle&)=&PDBWriter::Write;
void (PDBWriter::*write_b)(const mol::EntityView&)=&PDBWriter::Write;
+void remove_profiles() {
+ IOProfileRegistry::Instance().RemoveProfiles();
+}
+
void export_pdb_io()
{
class_<IOProfile>("IOProfile",
@@ -57,12 +61,13 @@ void export_pdb_io()
;
class_<IOProfileRegistry>("IOProfileRegistry", no_init)
.def("Get", &IOProfileRegistry::Get,
- return_value_policy<reference_existing_object>())
+ return_internal_reference<>())
.def("Set", &IOProfileRegistry::Set)
.def("Instance", &IOProfileRegistry::Instance,
+ //return_internal_reference).staticmethod("Instance")
return_value_policy<reference_existing_object>()).staticmethod("Instance")
.def("GetDefault", &IOProfileRegistry::GetDefault,
- return_value_policy<reference_existing_object>())
+ return_internal_reference<>())
;
class_<PDBReader, boost::noncopyable>("PDBReader", init<String, const IOProfile&>())
.def("HasNext", &PDBReader::HasNext)
@@ -83,4 +88,14 @@ void export_pdb_io()
&PDBWriter::SetWriteMultiModel)
.def("Write", write_b)
;
+
+ // we need to make sure there are no pending references to Python objects
+ // tied to the IOProfileRegistry singleton. The destructor of
+ // IOProfileRegistry may be called after Python is shutdown which results
+ // in a segfault.
+ scope().attr("__dict__")["atexit"]=handle<>(PyImport_ImportModule("atexit"));
+
+ def("_remove_profiles", &remove_profiles);
+ object r=scope().attr("_remove_profiles");
+ scope().attr("atexit").attr("register")(r);
}
diff --git a/modules/io/src/mol/io_profile.hh b/modules/io/src/mol/io_profile.hh
index abf37a6b2..188ed158b 100644
--- a/modules/io/src/mol/io_profile.hh
+++ b/modules/io/src/mol/io_profile.hh
@@ -49,7 +49,6 @@ public:
bool no_hetatms;
bool calpha_only;
conop::ProcessorPtr processor;
-
IOProfile Copy()
{
return IOProfile(dialect, quack_mode, fault_tolerant, join_spread_atom_records,
@@ -86,6 +85,9 @@ public:
}
IOProfile& GetDefault() { return profiles_["DEFAULT"]; }
+ void RemoveProfiles() {
+ profiles_.clear();
+ }
private:
IOProfileRegistry();
std::map<String, IOProfile> profiles_;
diff --git a/modules/io/tests/test_io_pdb.py b/modules/io/tests/test_io_pdb.py
index 9beb684d9..f3f3acbdf 100644
--- a/modules/io/tests/test_io_pdb.py
+++ b/modules/io/tests/test_io_pdb.py
@@ -11,6 +11,14 @@ class TestPDB(unittest.TestCase):
ch = e.FindChain("A");
self.assertEquals(ch.GetIntProp("mol_id"), 1)
+ def test_properly_assigns_profile_properties(self):
+ io.profiles['TEST'] = io.IOProfile()
+ io.profiles['TEST'].quack_mode = False
+ self.assertFalse(io.profiles['TEST'].quack_mode)
+ self.assertFalse(io.profiles['TEST'].Copy().quack_mode)
+ io.profiles['TEST'].quack_mode = True
+ self.assertTrue(io.profiles['TEST'].quack_mode)
+ self.assertTrue(io.profiles['TEST'].Copy().quack_mode)
def test_no_bond_feasibility(self):
io.profiles['FEAS_CHECK']=io.IOProfile(processor=conop.HeuristicProcessor(check_bond_feasibility=True))
io.profiles['NO_FEAS_CHECK']=io.IOProfile(processor=conop.HeuristicProcessor(check_bond_feasibility=False))
--
GitLab