diff --git a/src/proxy/plugin_python_en.ts b/src/proxy/plugin_python_en.ts index 4a3814e..99302dd 100644 --- a/src/proxy/plugin_python_en.ts +++ b/src/proxy/plugin_python_en.ts @@ -14,48 +14,48 @@ - + ModOrganizer path contains a semicolon - + Python DLL not found - + Invalid Python DLL - + Initializing Python failed - - + + invalid problem key %1 - + The path to Mod Organizer (%1) contains a semicolon. <br>While this is legal on NTFS drives, many softwares do not handle it correctly.<br>Unfortunately MO depends on libraries that seem to fall into that group.<br>As a result the python plugin cannot be loaded, and the only solution we canoffer is to remove the semicolon or move MO to a path without a semicolon. - + The Python plugin DLL was not found, maybe your antivirus deleted it. Re-installing MO2 might fix the problem. - + The Python plugin DLL is invalid, maybe your antivirus is blocking it. Re-installing MO2 and adding exclusions for it to your AV might fix the problem. - + The initialization of the Python plugin DLL failed, unfortunately without any details. diff --git a/src/proxy/proxypython.cpp b/src/proxy/proxypython.cpp index 0e43c36..035063c 100644 --- a/src/proxy/proxypython.cpp +++ b/src/proxy/proxypython.cpp @@ -169,12 +169,7 @@ QList ProxyPython::load(const QString& identifier) if (!m_Runner) { return {}; } - - auto plugins = m_Runner->load(identifier); - for (auto* plugin : plugins) { - plugin->setParent(this); - } - return plugins; + return m_Runner->load(identifier); } void ProxyPython::unload(const QString& identifier) diff --git a/src/runner-pybind11/pybind11_qt/pybind11_qt.h b/src/runner-pybind11/pybind11_qt/pybind11_qt.h index 4a33b07..206d7c7 100644 --- a/src/runner-pybind11/pybind11_qt/pybind11_qt.h +++ b/src/runner-pybind11/pybind11_qt/pybind11_qt.h @@ -10,6 +10,33 @@ namespace pybind11::qt { + /** + * @brief Tie the lifetime of the Python object to the lifetime of the given + * QObject. + * + * @param owner QObject that will own the python object. + * @param child Python object that the QObject will own. + */ + inline void set_owner(QObject* owner, object child) + { + new detail::qt::qobject_holder{owner, child}; + } + + /** + * @brief Tie the lifetime of the given object to the lifetime of the corresponding + * Python object. + * + * This object must have been created from Python and must inherit QObject. + * + * @param object Object to tie. + */ + template + void set_owner(Class* object) + { + static_assert(std::is_base_of_v); + new detail::qt::qobject_holder{object}; + } + /** * @brief Add Qt "delegate" to the given class. * diff --git a/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h b/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h index 6438c8c..29d6d8f 100644 --- a/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h +++ b/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h @@ -11,10 +11,18 @@ namespace pybind11::detail::qt { object p_; public: + /** + * @brief Construct a new qobject holder linked to the given QObject and + * maintaining the given python object alive. + * + * @param p Parent of this holder. + * @param o Python object to keep alive. + */ + qobject_holder(QObject* p, object o) : p_{o} { setParent(p); } + template - qobject_holder(U* p) : p_{reinterpret_borrow(cast(p))} + qobject_holder(U* p) : qobject_holder{p, reinterpret_borrow(cast(p))} { - setParent(p); } ~qobject_holder() diff --git a/src/runner-pybind11/pythonrunner.cpp b/src/runner-pybind11/pythonrunner.cpp index f402d1c..5b3dbe2 100644 --- a/src/runner-pybind11/pythonrunner.cpp +++ b/src/runner-pybind11/pythonrunner.cpp @@ -276,8 +276,8 @@ PYBIND11_MODULE(mobase, m) class PythonRunner : public IPythonRunner { public: - PythonRunner(); - ~PythonRunner(); + PythonRunner() = default; + ~PythonRunner() = default; bool initPython(); @@ -296,9 +296,10 @@ private: void ensureFolderInPath(QString folder); private: - // For each "identifier" (python file or python module folder), contains the - // list of python objects to keep "alive" during the execution. - std::unordered_map> m_PythonObjects; + // for each "identifier" (python file or python module folder), contains the + // list of python objects - this does not keep the objects alive, it simply used to + // unload plugins + std::unordered_map> m_PythonObjects; }; IPythonRunner* CreatePythonRunner() @@ -312,30 +313,6 @@ IPythonRunner* CreatePythonRunner() } } -PythonRunner::PythonRunner() {} - -PythonRunner::~PythonRunner() -{ - // Boost.Python does not handle cyclic garbace collection, so we need to release - // everything hold by the objects before deleting the objects themselves: - // for (auto& [name, objects] : m_PythonObjects) { - // for (auto& obj : objects) { - // obj.attr("__dict__").attr("clear")(); - // } - // } - - // we need to clear this here otherwise there is a crash in - // finalize_interpreter() - // { - // py::gil_scoped_acquire s; - // m_PythonObjects.clear(); - // } - - // py::finalize_interpreter(); -} - -// ErrWrapper is in error.h - PYBIND11_MODULE(moprivate, m) { // expose a function to create a particular tree, only for debugging @@ -352,17 +329,31 @@ bool PythonRunner::initPython() try { static const char* argv0 = "ModOrganizer.exe"; - PyImport_AppendInittab("mobase", &PyInit_mobase); - PyImport_AppendInittab("moprivate", &PyInit_moprivate); + initPath(); + + if (PyImport_AppendInittab("mobase", &PyInit_mobase) == -1) { + MOBase::log::error("failed to init python: failed to append mobase."); + return false; + } + + if (PyImport_AppendInittab("moprivate", &PyInit_moprivate) == -1) { + MOBase::log::error("failed to init python: failed to append moprivate."); + return false; + } Py_OptimizeFlag = 2; Py_NoSiteFlag = 1; - initPath(); - py::initialize_interpreter(false, 1, &argv0); if (!Py_IsInitialized()) { + MOBase::log::error( + "failed to init python: failed to initialize interpreter."); + + if (PyGILState_Check()) { + PyEval_SaveThread(); + } + return false; } @@ -375,6 +366,12 @@ bool PythonRunner::initPython() mo2::python::configure_python_stream(); mo2::python::configure_python_logging(mainNamespace["mobase"]); + // we need to release the GIL here - which is what this does + // + // when Python is initialized, the GIl is acquired, and if it is not release, + // trying to acquire it on a different thread will deadlock + PyEval_SaveThread(); + return true; } catch (const py::error_already_set& ex) { @@ -383,16 +380,6 @@ bool PythonRunner::initPython() } } -void PythonRunner::initPath() -{ - static QStringList paths = {QCoreApplication::applicationDirPath() + - "/pythoncore.zip", - QCoreApplication::applicationDirPath() + "/pythoncore", - IOrganizer::getPluginDataPath()}; - - Py_SetPath(paths.join(';').toStdWString().c_str()); -} - void PythonRunner::ensureFolderInPath(QString folder) { py::module_ sys = py::module_::import("sys"); @@ -406,6 +393,16 @@ void PythonRunner::ensureFolderInPath(QString folder) } } +void PythonRunner::initPath() +{ + static QStringList paths = {QCoreApplication::applicationDirPath() + + "/pythoncore.zip", + QCoreApplication::applicationDirPath() + "/pythoncore", + IOrganizer::getPluginDataPath()}; + + Py_SetPath(paths.join(';').toStdWString().c_str()); +} + QList PythonRunner::load(const QString& identifier) { py::gil_scoped_acquire lock; @@ -428,11 +425,14 @@ QList PythonRunner::load(const QString& identifier) py::dict moduleDict; if (identifier.endsWith(".py")) { - py::object mainModule = py::module_::import("__main__"); - py::dict moduleNamespace = mainModule.attr("__dict__"); + py::object mainModule = py::module_::import("__main__"); + + // make a copy, otherwise we might end up calling the createPlugin() or + // createPlugins() function multiple time + py::dict moduleNamespace = mainModule.attr("__dict__").attr("copy")(); std::string temp = ToString(identifier); - py::eval_file(temp.c_str(), moduleNamespace).is_none(); + py::eval_file(temp, moduleNamespace).is_none(); moduleDict = moduleNamespace; } else { @@ -442,7 +442,15 @@ QList PythonRunner::load(const QString& identifier) ensureFolderInPath(parts.join("/")); // check if the module is already loaded - moduleDict = py::module_::import(moduleName.c_str()).attr("__dict__"); + py::dict modules = py::module_::import("sys").attr("modules"); + if (modules.contains(moduleName)) { + py::module_ prev = modules[py::str(moduleName)]; + py::module_(prev).reload(); + moduleDict = prev.attr("__dict__"); + } + else { + moduleDict = py::module_::import(moduleName.c_str()).attr("__dict__"); + } } if (py::len(moduleDict) == 0) { @@ -455,9 +463,6 @@ QList PythonRunner::load(const QString& identifier) if (moduleDict.contains("createPlugin")) { plugins.push_back(moduleDict["createPlugin"]()); - - // Clear for future call - // PyDict_DelItemString(moduleDict.ptr(), "createPlugin"); } else if (moduleDict.contains("createPlugins")) { py::object pyPlugins = moduleDict["createPlugins"](); @@ -472,9 +477,6 @@ QList PythonRunner::load(const QString& identifier) plugins.push_back(pyList[i]); } } - - // Clear for future call - // PyDict_DelItemString(moduleDict.ptr(), "createPlugins"); } else { MOBase::log::error("Plugin {}: missing a createPlugin(s) function.", @@ -491,7 +493,7 @@ QList PythonRunner::load(const QString& identifier) for (py::object pluginObj : plugins) { - // Add the plugin to keep it alive: + // save to be able to unload it m_PythonObjects[identifier].push_back(pluginObj); QList interfaceList = mo2::python::extract_plugins(pluginObj); @@ -501,6 +503,11 @@ QList PythonRunner::load(const QString& identifier) identifier); } + // tie the lifetime of the Python object to the lifetime of the QObject + for (auto* object : interfaceList) { + py::qt::set_owner(object, pluginObj); + } + // Append the plugins to the main list: allInterfaceList.append(interfaceList); }