Merge PR #486: Fix a double-free when shutting down the python engine.

libnymea-core/integrations/pythonintegrationplugin.cpp

@@ -242,7 +242,6 @@ PythonIntegrationPlugin::~PythonIntegrationPlugin()
     s_plugins.take(this);
     Py_XDECREF(m_pluginModule);
     Py_DECREF(m_nymeaModule);
-    Py_XDECREF(m_logger);
     Py_XDECREF(m_stdOutHandler);
     Py_XDECREF(m_stdErrHandler);
 
@@ -259,11 +258,13 @@ void PythonIntegrationPlugin::initPython()
     // Only modify the init tab once (initPython() might be called again after calling deinitPython())
     static bool initTabPrepared = false;
     if (!initTabPrepared) {
+        qCDebug(dcPythonIntegrations()) << "Adding nymea module to init tab";
         PyImport_AppendInittab("nymea", PyInit_nymea);
         initTabPrepared = true;
     }
 
     // Initialize the python engine and fire up threading support
+    qCDebug(dcPythonIntegrations()) << "Initializing Python engine";
     Py_InitializeEx(0);
     PyEval_InitThreads();
 
@@ -277,6 +278,7 @@ void PythonIntegrationPlugin::deinitPython()
     PyEval_RestoreThread(s_mainThreadState);
 
     // Tear down the python engine
+    qCDebug(dcPythonIntegrations()) << "Finalizing Python engine";
     Py_Finalize();
 
     // Our main thread state is destroyed now
@@ -289,6 +291,7 @@ bool PythonIntegrationPlugin::loadScript(const QString &scriptFile)
     PyEval_RestoreThread(s_mainThreadState);
 
     // Create a new interpreter
+    qCDebug(dcPythonIntegrations()) << "Creatig new Python interpreter for script:" << scriptFile;
     m_threadState = Py_NewInterpreter();
 
     // Switch to the new interpreter thread state
@@ -361,12 +364,19 @@ bool PythonIntegrationPlugin::loadScript(const QString &scriptFile)
     s_plugins.insert(this, m_pluginModule);
 
     // Set up logger with appropriate logging category
+    // Note: AddModule steals the reference and will delete it on Interpreter shutdown, so not keeping a reference to the logger.
     QString category = metadata.pluginName();
     category.replace(0, 1, category[0].toUpper());
     PyObject *args = Py_BuildValue("(s)", category.toUtf8().data());
-    m_logger = PyObject_CallObject((PyObject*)&PyNymeaLoggingHandlerType, args);
+    PyObject *logger = PyObject_CallObject((PyObject*)&PyNymeaLoggingHandlerType, args);
     Py_DECREF(args);
 
+    int loggerAdded = PyModule_AddObject(m_pluginModule, "logger", logger);
+    if (loggerAdded != 0) {
+        qCWarning(dcPythonIntegrations()) << "Failed to add the logger object";
+        Py_DECREF(logger);
+    }
+
     // Override stdout and stderr
     args = Py_BuildValue("(si)", category.toUtf8().data(), QtMsgType::QtDebugMsg);
     m_stdOutHandler = PyObject_CallObject((PyObject*)&PyStdOutHandlerType, args);
@@ -377,12 +387,6 @@ bool PythonIntegrationPlugin::loadScript(const QString &scriptFile)
     PySys_SetObject("stderr", m_stdErrHandler);
     Py_DECREF(args);
 
-    int loggerAdded = PyModule_AddObject(m_pluginModule, "logger", m_logger);
-    if (loggerAdded != 0) {
-        qCWarning(dcPythonIntegrations()) << "Failed to add the logger object";
-        Py_DECREF(m_logger);
-        m_logger = nullptr;
-    }
 
     // Export metadata ids into module
     exportIds();

libnymea-core/integrations/pythonintegrationplugin.h

@@ -81,7 +81,6 @@ private:
     PyObject *m_nymeaModule = nullptr;
     // The imported plugin module (the plugin.py)
     PyObject *m_pluginModule = nullptr;
-    PyObject *m_logger = nullptr;
     PyObject *m_stdOutHandler = nullptr;
     PyObject *m_stdErrHandler = nullptr;
 

tests/auto/pythonplugins/testpythonplugins.cpp

@@ -51,17 +51,22 @@ private slots:
 
     void initTestCase();
 
+    void testRestartServer();
+
     void setupAndRemoveThing();
     void testDiscoverPairAndRemoveThing();
 
 
 };
 
+void TestPythonPlugins::testRestartServer()
+{
+    NymeaTestBase::restartServer();
+}
 
 void TestPythonPlugins::initTestCase()
 {
-    NymeaTestBase::initTestCase();
+    NymeaTestBase::initTestCase("*.debug=false\n*.info=false\n*.warning=false\n"
-    QLoggingCategory::setFilterRules("*.debug=false\n"
                                      "Tests.debug=true\n"
                                      "PyMock.debug=true\n"
                                      "PythonIntegrations.debug=true\n"

tests/testlib/nymeatestbase.cpp

@@ -53,7 +53,7 @@ NymeaTestBase::NymeaTestBase(QObject *parent) :
     QCoreApplication::instance()->setOrganizationName("nymea-test");
 }
 
-void NymeaTestBase::initTestCase()
+void NymeaTestBase::initTestCase(const QString &loggingRules)
 {
     qCDebug(dcTests) << "NymeaTestBase starting.";
 
@@ -71,7 +71,11 @@ void NymeaTestBase::initTestCase()
     NymeaSettings nymeadSettings(NymeaSettings::SettingsRoleGlobal);
     nymeadSettings.clear();
 
-    QLoggingCategory::setFilterRules("*.debug=false\nApplication.debug=true\nTests.debug=true\nMock.debug=true");
+    if (loggingRules.isEmpty()) {
+        QLoggingCategory::setFilterRules("*.debug=false\nApplication.debug=true\nTests.debug=true\nMock.debug=true");
+    } else {
+        QLoggingCategory::setFilterRules(loggingRules);
+    }
 
     // Start the server
     qCDebug(dcTests()) << "Setting up nymea core instance";

tests/testlib/nymeatestbase.h

@@ -50,7 +50,7 @@ public:
     explicit NymeaTestBase(QObject *parent = nullptr);
 
 protected slots:
-    void initTestCase();
+    void initTestCase(const QString &loggingRules = QString());
     void cleanupTestCase();
     void cleanup();