diff --git a/src/proxy/proxypython.cpp b/src/proxy/proxypython.cpp index da98fa2..47691ed 100644 --- a/src/proxy/proxypython.cpp +++ b/src/proxy/proxypython.cpp @@ -104,6 +104,8 @@ ProxyPython::ProxyPython() ProxyPython::~ProxyPython() { + delete m_Runner; + if (!m_TempRunnerFile.isEmpty()) { ::FreeLibrary(m_RunnerLib); QFile(m_TempRunnerFile).remove(); diff --git a/src/runner/CMakeLists.txt b/src/runner/CMakeLists.txt index cac7f40..6c4a506 100644 --- a/src/runner/CMakeLists.txt +++ b/src/runner/CMakeLists.txt @@ -23,3 +23,29 @@ endif() requires_project(game_features) requires_library(python) + +add_filter(NAME src/converters GROUPS + converters + pythonutils + shared_ptr_converter + tuple_helper + variant_helper +) + +add_filter(NAME src/runner GROUPS + pythonrunner + pylogger +) + +add_filter(NAME src/utils GROUPS + error + gilock + sipapiaccess +) + +add_filter(NAME src/wrappers GROUPS + gamefeatureswrappers + proxypluginwrappers + pythonwrappersutilities + uibasewrappers +) diff --git a/src/runner/pythonrunner.cpp b/src/runner/pythonrunner.cpp index 9366843..feda706 100644 --- a/src/runner/pythonrunner.cpp +++ b/src/runner/pythonrunner.cpp @@ -39,6 +39,7 @@ #include "tuple_helper.h" #include "variant_helper.h" #include "converters.h" +#include "shared_ptr_converter.h" #include "pylogger.h" using namespace MOBase; @@ -86,6 +87,7 @@ BOOST_PYTHON_MODULE(mobase) utils::register_qflags_converter(); // Pointers: + utils::shared_ptr_from_python>(); bpy::register_ptr_to_python>(); bpy::register_ptr_to_python>(); bpy::implicitly_convertible, std::shared_ptr>(); @@ -254,7 +256,7 @@ BOOST_PYTHON_MODULE(mobase) .def("process", &ExecutableForcedLoadSetting::process) ; - bpy::class_, std::shared_ptr, boost::noncopyable>("ISaveGame") + bpy::class_, boost::noncopyable>("ISaveGame") .def("getFilepath", bpy::pure_virtual(&ISaveGame::getFilepath)) .def("getCreationTime", bpy::pure_virtual(&ISaveGame::getCreationTime)) .def("getName", bpy::pure_virtual(&ISaveGame::getName)) @@ -1101,6 +1103,8 @@ class PythonRunner : public IPythonRunner public: PythonRunner(); + ~PythonRunner(); + bool initPython(const QString& pythonDir); QList instantiate(const QString& pluginName); bool isPythonInstalled() const; @@ -1151,6 +1155,12 @@ PythonRunner::PythonRunner() m_PythonHome = new wchar_t[MAX_PATH + 1]; } +PythonRunner::~PythonRunner() { + // We need the GIL lock when destroying Python objects. + GILock lock; + m_PythonObjects.clear(); +} + static const char *argv0 = "ModOrganizer.exe"; struct PrintWrapper diff --git a/src/runner/pythonrunner.h b/src/runner/pythonrunner.h index 73de994..c494ae1 100644 --- a/src/runner/pythonrunner.h +++ b/src/runner/pythonrunner.h @@ -13,6 +13,8 @@ public: virtual QList instantiate(const QString &pluginName) = 0; virtual bool isPythonInstalled() const = 0; virtual bool isPythonVersionSupported() const = 0; + + virtual ~IPythonRunner() { } }; diff --git a/src/runner/shared_ptr_converter.h b/src/runner/shared_ptr_converter.h new file mode 100644 index 0000000..e49d683 --- /dev/null +++ b/src/runner/shared_ptr_converter.h @@ -0,0 +1,99 @@ +#ifndef PYTHONRUNNER_SHARED_PTR_CONVERTER_H +#define PYTHONRUNNER_SHARED_PTR_CONVERTER_H + +#include + +#include "error.h" +#include "gilock.h" + +namespace utils { + + // Shared pointers are handled in a special way by Boost.Python since they hold + // the wrapped Python object and only release it when the ref counter of the shared + // ptr drops to 0 using shared_ptr_deleter. + // + // Unfortunately for us, this will happen outside of the Python proxy for some objects + // and thus without the GIL lock, making everything crash, so we need a custom deleter + // that holds the GIL while releasing the lock. + // + // Note that this is only useful for Python -> C++ conversion, and without this, Boost + // will automatically wrapped the pointer. The C++ -> Python conversion is handled + // separately by boost::python::register_ptr_to_python. + + template + struct shared_ptr_from_python; + + namespace details { + + struct shared_ptr_deleter_with_gil_lock : boost::python::converter::shared_ptr_deleter { + + using shared_ptr_deleter::shared_ptr_deleter; + + void operator()(void const* o) { + GILock lock; + shared_ptr_deleter::operator()(o); + } + + }; + + template + struct shared_ptr_void; + + template + struct shared_ptr_void> { using type = std::shared_ptr; }; + + template + struct shared_ptr_void> { using type = boost::shared_ptr; }; + + template + using shared_ptr_void_t = typename shared_ptr_void::type; + + } + + template + struct shared_ptr_from_python + { + using T = typename SharedPtr::element_type; + + shared_ptr_from_python() + { + using namespace boost::python; + converter::registry::insert(&convertible, &construct, type_id() +#ifndef BOOST_PYTHON_NO_PY_SIGNATURES + , &converter::expected_from_python_type_direct::get_pytype +#endif + ); + } + + private: + static void* convertible(PyObject* p) + { + if (p == Py_None) + return p; + + return boost::python::converter::get_lvalue_from_python(p, boost::python::converter::registered::converters); + } + + static void construct(PyObject* source, boost::python::converter::rvalue_from_python_stage1_data* data) + { + using namespace boost::python; + void* const storage = ((converter::rvalue_from_python_storage*)data)->storage.bytes; + // Deal with the "None" case. + if (data->convertible == source) + new (storage) SharedPtr(); + else + { + details::shared_ptr_void_t hold_convertible_ref_count( + (void*)0, details::shared_ptr_deleter_with_gil_lock(handle<>(borrowed(source)))); + // use aliasing constructor + new (storage) SharedPtr(hold_convertible_ref_count, + static_cast(data->convertible)); + } + + data->convertible = storage; + } + }; + +} + +#endif \ No newline at end of file diff --git a/src/runner/uibasewrappers.h b/src/runner/uibasewrappers.h index ace9f0f..95e878a 100644 --- a/src/runner/uibasewrappers.h +++ b/src/runner/uibasewrappers.h @@ -50,8 +50,6 @@ public: protected: friend class IPluginGameWrapper; - mutable boost::python::object m_PySave; - }; // This needs a wrapper but currently I have no idea how to expose this properly to python: