Merge pull request #1858 from KLayout/bugfix/issue-1857

Bugfix/issue 1857

src/layview/layview/layLayoutView_qt.cc

@@ -650,6 +650,11 @@ void LayoutView::close()
     ms_current = 0;
   }
 
+  if (mp_properties_dialog) {
+    //  must happen before "shutdown" (issue #1857)
+    delete mp_properties_dialog.data ();
+  }
+
   //  release all components and plugins before we delete the user interface
   shutdown ();
 
@@ -687,10 +692,6 @@ void LayoutView::close()
   }
   mp_bookmarks_frame = 0;
   mp_bookmarks_view = 0;
-
-  if (mp_properties_dialog) {
-    delete mp_properties_dialog.data ();
-  }
 }
 
 void

src/pya/pya/pya.cc

@@ -192,7 +192,7 @@ PythonInterpreter::PythonInterpreter (bool embedded)
 
     sp_interpreter = this;
 
-    //  this monitor whether Python shuts down and deletes the interpreter's
+    //  this monitors whether Python shuts down and deletes the interpreter's
     //  instance.
     //  NOTE: this assumes, the interpreter was created with new(!)
     Py_AtExit (&reset_interpreter);
@@ -360,6 +360,12 @@ PythonInterpreter::PythonInterpreter (bool embedded)
 
 PythonInterpreter::~PythonInterpreter ()
 {
+  for (auto m = m_modules.begin (); m != m_modules.end (); ++m) {
+    (*m)->cleanup ();
+  }
+
+  PYAObjectBase::clear_callbacks_cache (m_embedded);
+
   m_stdout_channel = PythonRef ();
   m_stderr_channel = PythonRef ();
   m_stdout = PythonPtr ();
@@ -370,6 +376,23 @@ PythonInterpreter::~PythonInterpreter ()
   if (m_embedded) {
     Py_Finalize ();
   }
+
+  for (auto m = m_modules.begin (); m != m_modules.end (); ++m) {
+    delete *m;
+  }
+  m_modules.clear ();
+}
+
+void
+PythonInterpreter::register_module (pya::PythonModule *module)
+{
+  for (auto m = m_modules.begin (); m != m_modules.end (); ++m) {
+    if (*m == module) {
+      return; //  already registered
+    }
+  }
+
+  m_modules.push_back (module);
 }
 
 char *

src/pya/pya/pya.h

@@ -105,6 +105,14 @@ public:
    */
   ~PythonInterpreter ();
 
+  /**
+   *  @brief Registers a module
+   *
+   *  The registered modules are cleaned up before the interpreter shuts down. The interpreter takes
+   *  ownership of the module object.
+   */
+  void register_module (pya::PythonModule *module);
+
   /**
    *  @brief Add the given path to the search path
    */
@@ -279,7 +287,7 @@ private:
   std::map<PyObject *, size_t> m_file_id_map;
   std::wstring mp_py3_app_name;
   bool m_embedded;
-  std::unique_ptr<pya::PythonModule> m_pya_module;
+  std::vector<pya::PythonModule *> m_modules;
 };
 
 }

src/pya/pya/pyaCallables.cc

@@ -51,6 +51,9 @@ pya_object_deallocate (PyObject *self)
   //  we better work around it.
   ++self->ob_refcnt;
 
+  //  Mute Python warnings in debug case
+  PyObject_GC_UnTrack (self);
+
   PYAObjectBase *p = PYAObjectBase::from_pyobject (self);
   p->~PYAObjectBase ();
   Py_TYPE (self)->tp_free (self);

src/pya/pya/pyaModule.cc

@@ -71,12 +71,6 @@ PythonModule::PythonModule ()
 
 PythonModule::~PythonModule ()
 {
-  PYAObjectBase::clear_callbacks_cache ();
-
-  //  the Python objects were probably deleted by Python itself as it exited -
-  //  don't try to delete them again.
-  mp_module.release ();
-
   while (!m_methods_heap.empty ()) {
     delete m_methods_heap.back ();
     m_methods_heap.pop_back ();
@@ -93,6 +87,14 @@ PythonModule::~PythonModule ()
   }
 }
 
+void
+PythonModule::cleanup ()
+{
+  //  the Python objects are probably deleted by Python itself as it exits -
+  //  don't try to delete them again in the destructor.
+  mp_module.release ();
+}
+
 PyObject *
 PythonModule::module ()
 {

src/pya/pya/pyaModule.h

@@ -65,6 +65,12 @@ public:
    */
   ~PythonModule ();
 
+  /**
+   *  @brief Clean up the module
+   *  This method is called by the interpreter before Py_Finalize
+   */
+  void cleanup ();
+
   /**
    *  @brief Initializes the module
    *  This entry point is for external use where the module has not been created yet

src/pya/pya/pyaObject.cc

@@ -514,8 +514,16 @@ PYAObjectBase::initialize_callbacks ()
 }
 
 void 
-PYAObjectBase::clear_callbacks_cache ()
+PYAObjectBase::clear_callbacks_cache (bool embedded)
 {
+  //  if not embedded, we cannot use the python API at this stage - do not try to
+  //  reference count the objects there.
+  if (! embedded) {
+    for (auto c = s_callbacks_cache.begin (); c != s_callbacks_cache.end (); ++c) {
+      c->first.release_const ();
+    }
+  }
+
   s_callbacks_cache.clear ();
 }
 

src/pya/pya/pyaObject.h

@@ -200,7 +200,7 @@ public:
   /**
    *  @brief Clears the callbacks cache
    */
-  static void clear_callbacks_cache ();
+  static void clear_callbacks_cache (bool embedded);
 
 private:
   friend class StatusChangedListener;

src/pya/pya/pyaRefs.cc

@@ -32,19 +32,19 @@ namespace pya
 //  PythonRef implementation
 
 PythonRef::PythonRef ()
-  : mp_obj (NULL)
+  : mp_obj (NULL), m_owns_pointer (true)
 { 
   //  .. nothing yet ..
 }
 
 PythonRef::PythonRef (const PythonPtr &ptr)
-  : mp_obj (ptr.get ())
+  : mp_obj (ptr.get ()), m_owns_pointer (true)
 { 
   Py_XINCREF (mp_obj);
 }
 
 PythonRef::PythonRef (PyObject *obj, bool new_ref)
-  : mp_obj (obj)
+  : mp_obj (obj), m_owns_pointer (true)
 { 
   if (! new_ref) {
     Py_XINCREF (mp_obj);
@@ -53,38 +53,49 @@ PythonRef::PythonRef (PyObject *obj, bool new_ref)
 
 PythonRef &PythonRef::operator= (PyObject *obj)
 {
-  Py_XDECREF (mp_obj);
+  if (m_owns_pointer) {
+    Py_XDECREF (mp_obj);
+  }
   mp_obj = obj;
+  m_owns_pointer = true;
   return *this;
 }
 
 PythonRef &PythonRef::operator= (const PythonPtr &ptr)
 {
-  Py_XDECREF (mp_obj);
+  if (m_owns_pointer) {
+    Py_XDECREF (mp_obj);
+  }
   mp_obj = ptr.get ();
   Py_XINCREF (mp_obj);
+  m_owns_pointer = true;
   return *this;
 }
 
 PythonRef &PythonRef::operator= (const PythonRef &other)
 {
   if (this != &other && mp_obj != other.mp_obj) {
-    Py_XDECREF (mp_obj);
+    if (m_owns_pointer) {
+      Py_XDECREF (mp_obj);
+    }
     mp_obj = other.mp_obj;
+    m_owns_pointer = true;
     Py_XINCREF (mp_obj);
   }
   return *this;
 }
 
 PythonRef::PythonRef (const PythonRef &other)
-  : mp_obj (other.mp_obj)
+  : mp_obj (other.mp_obj), m_owns_pointer (true)
 {
   Py_XINCREF (mp_obj);
 }
 
 PythonRef::~PythonRef ()
 {
-  Py_XDECREF (mp_obj);
+  if (m_owns_pointer) {
+    Py_XDECREF (mp_obj);
+  }
 }
 
 PythonRef::operator bool () const
@@ -109,6 +120,12 @@ PyObject *PythonRef::release ()
   return o;
 }
 
+PyObject *PythonRef::release_const () const
+{
+  m_owns_pointer = false;
+  return mp_obj;
+}
+
 // --------------------------------------------------------------------------
 //  PythonPtr implementation
 

src/pya/pya/pyaRefs.h

@@ -116,6 +116,14 @@ public:
    */
   PyObject *release ();
 
+  /**
+   *  @brief Takes the pointer, but does not change the value
+   *  This method will stop the reference from managing the object, but
+   *  maintains the pointer. Do not access the pointer after this
+   *  operation.
+   */
+  PyObject *release_const () const;
+
   /**
    *  @brief Comparison operator
    */
@@ -134,6 +142,7 @@ public:
 
 private:
   PyObject *mp_obj;
+  mutable bool m_owns_pointer;
 };
 
 /**

src/pymod/pymodHelper.h

@@ -34,6 +34,7 @@
 
 #include "pyaModule.h"
 #include "pyaUtils.h"
+#include "pya.h"
 
 #include "gsi.h"
 #include "gsiExpression.h"
@@ -41,7 +42,7 @@
 static PyObject *
 module_init (const char *pymod_name, const char *mod_name, const char *mod_description)
 {
-  static pya::PythonModule module;
+  std::unique_ptr<pya::PythonModule> module (new pya::PythonModule ());
 
   PYA_TRY
   
@@ -50,13 +51,18 @@ module_init (const char *pymod_name, const char *mod_name, const char *mod_descr
     //  required for the tiling processor for example
     gsi::initialize_expressions ();
 
-    module.init (pymod_name, mod_description);
-    module.make_classes (mod_name);
+    module->init (pymod_name, mod_description);
+    module->make_classes (mod_name);
 
-    return module.take_module ();
+    PyObject *mod_object = module->take_module ();
+
+    tl_assert (pya::PythonInterpreter::instance () != 0);
+    pya::PythonInterpreter::instance ()->register_module (module.release ());
+
+    return mod_object;
 
   PYA_CATCH_ANYWHERE
-  
+
   return 0;
 }