From 16c4fa334591404dac77c7c690819587f2cd1d43 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Mon, 31 Aug 2020 19:29:35 +0200 Subject: [PATCH] fix issues in teardown. --- .../python/pynymealogginghandler.h | 3 +- libnymea-core/integrations/python/pyparam.h | 1 - .../integrations/pythonintegrationplugin.cpp | 34 ++++++++++++++----- .../integrations/pythonintegrationplugin.h | 2 -- tests/auto/integrations/testintegrations.cpp | 1 + tests/testlib/nymeatestbase.cpp | 2 ++ 6 files changed, 30 insertions(+), 13 deletions(-) diff --git a/libnymea-core/integrations/python/pynymealogginghandler.h b/libnymea-core/integrations/python/pynymealogginghandler.h index a31d8213..2bcce1d4 100644 --- a/libnymea-core/integrations/python/pynymealogginghandler.h +++ b/libnymea-core/integrations/python/pynymealogginghandler.h @@ -21,6 +21,7 @@ typedef struct { static int PyNymeaLoggingHandler_init(PyNymeaLoggingHandler *self, PyObject *args, PyObject */*kwds*/) { + qCDebug(dcPythonIntegrations()) << "+++ PyNymeaLoggingHandler"; char *category = nullptr; if (!PyArg_ParseTuple(args, "s", &category)) { qCWarning(dcPythonIntegrations()) << "PyNymeaLoggingHandler: Error parsing parameters"; @@ -35,11 +36,11 @@ static int PyNymeaLoggingHandler_init(PyNymeaLoggingHandler *self, PyObject *arg static void PyNymeaLoggingHandler_dealloc(PyNymeaLoggingHandler * self) { + qCDebug(dcPythonIntegrations()) << "--- PyNymeaLoggingHandler"; free(self->category); Py_TYPE(self)->tp_free(self); } - static PyObject * PyNymeaLoggingHandler_log(PyNymeaLoggingHandler* self, PyObject* args) { QStringList strings; diff --git a/libnymea-core/integrations/python/pyparam.h b/libnymea-core/integrations/python/pyparam.h index 8278c343..ac37583e 100644 --- a/libnymea-core/integrations/python/pyparam.h +++ b/libnymea-core/integrations/python/pyparam.h @@ -67,7 +67,6 @@ static PyTypeObject PyParamType = { sizeof(PyParam), /* tp_basicsize */ 0, /* tp_itemsize */ (destructor)PyParam_dealloc,/* tp_dealloc */ - }; static PyParam* PyParam_fromParam(const Param ¶m) diff --git a/libnymea-core/integrations/pythonintegrationplugin.cpp b/libnymea-core/integrations/pythonintegrationplugin.cpp index 6b09d409..dae5f754 100644 --- a/libnymea-core/integrations/pythonintegrationplugin.cpp +++ b/libnymea-core/integrations/pythonintegrationplugin.cpp @@ -212,7 +212,6 @@ PythonIntegrationPlugin::~PythonIntegrationPlugin() s_plugins.take(this); Py_XDECREF(m_pluginModule); - Py_XDECREF(m_logger); Py_DECREF(m_nymeaModule); Py_EndInterpreter(m_threadState); @@ -223,19 +222,33 @@ PythonIntegrationPlugin::~PythonIntegrationPlugin() void PythonIntegrationPlugin::initPython() { - Q_ASSERT_X(s_mainThreadState == nullptr, "PythonIntegrationPlugin::initPython()", "initPython() must be called exactly once."); + Q_ASSERT_X(s_mainThreadState == nullptr, "PythonIntegrationPlugin::initPython()", "initPython() must be called exactly once before calling deinitPython()."); - PyImport_AppendInittab("nymea", PyInit_nymea); + // Only modify the init tab once (initPython() might be called again after calling deinitPython()) + static bool initTabPrepared = false; + if (!initTabPrepared) { + PyImport_AppendInittab("nymea", PyInit_nymea); + initTabPrepared = true; + } + + // Initialize the python engine and fire up threading support Py_InitializeEx(0); PyEval_InitThreads(); + // Store the main thread state and release the GIL s_mainThreadState = PyEval_SaveThread(); } void PythonIntegrationPlugin::deinitPython() { + // Restore our main thread state PyEval_RestoreThread(s_mainThreadState); - Py_Finalize(); + + // Tear down the python engine + Py_FinalizeEx(); + + // Our main thread state is destroyed now + s_mainThreadState = nullptr; } bool PythonIntegrationPlugin::loadScript(const QString &scriptFile) @@ -244,18 +257,18 @@ bool PythonIntegrationPlugin::loadScript(const QString &scriptFile) QFile metaDataFile(fi.absolutePath() + "/" + fi.baseName() + ".json"); if (!metaDataFile.open(QFile::ReadOnly)) { - qCWarning(dcThingManager()) << "Error opening metadata file:" << metaDataFile.fileName(); + qCWarning(dcPythonIntegrations()) << "Error opening metadata file:" << metaDataFile.fileName(); return false; } QJsonParseError error; QJsonDocument jsonDoc = QJsonDocument::fromJson(metaDataFile.readAll(), &error); if (error.error != QJsonParseError::NoError) { - qCWarning(dcThingManager()) << "Error parsing metadata file:" << error.errorString(); + qCWarning(dcPythonIntegrations()) << "Error parsing metadata file:" << error.errorString(); return false; } setMetaData(PluginMetadata(jsonDoc.object())); if (!metadata().isValid()) { - qCWarning(dcThingManager()) << "Plugin metadata not valid for plugin:" << scriptFile; + qCWarning(dcPythonIntegrations()) << "Plugin metadata not valid for plugin:" << scriptFile; return false; } @@ -314,8 +327,11 @@ bool PythonIntegrationPlugin::loadScript(const QString &scriptFile) PyObject *args = Py_BuildValue("(s)", category.toUtf8().data()); PyNymeaLoggingHandler *logger = reinterpret_cast(PyObject_CallObject((PyObject*)&PyNymeaLoggingHandlerType, args)); Py_DECREF(args); - PyModule_AddObject(m_pluginModule, "logger", reinterpret_cast(logger)); - m_logger = (PyObject*)logger; + int loggerAdded = PyModule_AddObject(m_pluginModule, "logger", reinterpret_cast(logger)); + if (loggerAdded != 0) { + qCWarning(dcPythonIntegrations()) << "Failed to add the logger object"; + Py_DECREF(logger); + } // Export metadata ids into module exportIds(); diff --git a/libnymea-core/integrations/pythonintegrationplugin.h b/libnymea-core/integrations/pythonintegrationplugin.h index 1539202f..37bcaf28 100644 --- a/libnymea-core/integrations/pythonintegrationplugin.h +++ b/libnymea-core/integrations/pythonintegrationplugin.h @@ -76,8 +76,6 @@ private: // The imported plugin module (the plugin.py) PyObject *m_pluginModule = nullptr; - PyObject *m_logger = nullptr; - // A map of plugin instances to plugin python scripts/modules // Make sure to hold the GIL when accessing this. static QHash s_plugins; diff --git a/tests/auto/integrations/testintegrations.cpp b/tests/auto/integrations/testintegrations.cpp index b3c6e373..5b5d8039 100644 --- a/tests/auto/integrations/testintegrations.cpp +++ b/tests/auto/integrations/testintegrations.cpp @@ -161,6 +161,7 @@ void TestIntegrations::initTestCase() "Tests.debug=true\n" "Mock.debug=true\n" "Translations.debug=true\n" + "PythonIntegrations.debug=true\n" ); // Adding an async mock to be used in tests below diff --git a/tests/testlib/nymeatestbase.cpp b/tests/testlib/nymeatestbase.cpp index d124b865..cdc687f6 100644 --- a/tests/testlib/nymeatestbase.cpp +++ b/tests/testlib/nymeatestbase.cpp @@ -414,7 +414,9 @@ void NymeaTestBase::waitForDBSync() void NymeaTestBase::restartServer() { // Destroy and recreate the core instance... + qCDebug(dcTests()) << "Tearing down server instance"; NymeaCore::instance()->destroy(); + qCDebug(dcTests()) << "Restarting server instance"; NymeaCore::instance()->init(); QSignalSpy coreSpy(NymeaCore::instance(), SIGNAL(initialized())); coreSpy.wait();