diff --git a/pythonfmu/tests/test_integration.py b/pythonfmu/tests/test_integration.py index 001b195..118d1d2 100644 --- a/pythonfmu/tests/test_integration.py +++ b/pythonfmu/tests/test_integration.py @@ -354,3 +354,43 @@ def test_integration_throw_py_error(tmp_path): with pytest.raises(Exception): fmpy.simulate_fmu(str(fmu), stop_time=1.0) + + +@pytest.mark.integration +def test_integration_reinstantiate_same_fmu_keeps_module_dict_alive(tmp_path): + """findClass() must not Py_DECREF the borrowed PyModule_GetDict() result. + + The FMU runs inside this interpreter, so the reference count of the slave + module's __dict__ can be compared with the objects that visibly refer to it. + A stolen reference shows up as a count lower than the referrer count, one per + fmi2Instantiate of the same FMU. + """ + import gc + import sys + + script_file = Path(__file__).parent / "slaves/pythonslave.py" + fmu = FmuBuilder.build_FMU(script_file, dest=tmp_path, needsExecutionTool="false") + md = fmpy.read_model_description(fmu, validate=False) + unzip_dir = fmpy.extract(fmu) + + models = [] + for i in range(3): + model = fmpy.fmi2.FMU2Slave( + guid=md.guid, + unzipDirectory=unzip_dir, + modelIdentifier=md.coSimulation.modelIdentifier, + instanceName=f"instance{i}", + ) + model.instantiate() + models.append(model) + + gc.collect() + module_dict = sys.modules[script_file.stem].__dict__ + referrers = gc.get_referrers(module_dict) + # getrefcount() counts its own argument; every visible referrer must own a reference. + assert sys.getrefcount(module_dict) - 1 >= len(referrers) + + for model in models: + model.terminate() + model.fmi2FreeInstance(model.component) + model.component = None diff --git a/src/pythonfmu/PySlaveInstance.cpp b/src/pythonfmu/PySlaveInstance.cpp index f05c866..4965f8e 100644 --- a/src/pythonfmu/PySlaveInstance.cpp +++ b/src/pythonfmu/PySlaveInstance.cpp @@ -62,12 +62,10 @@ PyObject* findClass(const std::string& resources, const std::string& moduleName) PyObject* pResult = PyEval_EvalCode(pCode, pGlobals, pLocals); Py_XDECREF(pResult); } else { - PyErr_Print(); // Handle compilation error - Py_Finalize(); - Py_DECREF(pGlobals); + // Compilation failed: leave the exception set so the caller reports it + // through handle_py_exception(). Py_DECREF(pyModule); Py_DECREF(pLocals); - Py_DECREF(pCode); file.close(); return nullptr; } @@ -113,7 +111,7 @@ PyObject* findClass(const std::string& resources, const std::string& moduleName) Py_DECREF(pCode); Py_DECREF(pLocals); Py_DECREF(pyModule); - Py_DECREF(pGlobals); + // Borrowed reference (PyModule_GetDict); do not DECREF. file.close(); Py_DECREF(pyClassName); return pyClass; @@ -664,21 +662,26 @@ namespace { std::mutex pyStateMutex{}; -std::shared_ptr pyState{}; +// Intentionally heap-allocated and never deleted. A namespace-scope +// std::shared_ptr is destroyed by its static destructor at process exit +// and released again by the platform unload hook; the order of those two is +// unspecified, so the second release read a freed control block +// (heap-use-after-free -> "corrupted double-linked list" at exit). +std::shared_ptr* pyState = new std::shared_ptr(); } // namespace std::unique_ptr pythonfmu::createInstance(fmu_data data) { { - auto const ensurePyStateAlive = [&]() { + auto const ensurePyStateAlive = [&]() -> std::shared_ptr { auto const lock = std::lock_guard{pyStateMutex}; - if (nullptr == pyState) pyState = std::make_shared(); + if (nullptr == *pyState) *pyState = std::make_shared(); + return *pyState; }; - ensurePyStateAlive(); + data.pyState = ensurePyStateAlive(); auto c = std::make_unique(data); - data.pyState = pyState; return c; } } @@ -689,10 +692,11 @@ extern "C" { // The PyState instance owns it's own thread for constructing and destroying the Py* from the same thread. // Creation of an std::thread increments ref counter of a shared library. So, when the client unloads the library // the library won't be freed, as std::thread is alive, and the std::thread itself waits for de-initialization request. -// Thus, use DllMain on Windows and __attribute__((destructor)) on Linux for signaling to the PyState about de-initialization. +// Thus, use the platform unload hook to signal PyState about de-initialization. void finalizePythonInterpreter() { - pyState = nullptr; + auto const lock = std::lock_guard{pyStateMutex}; + pyState->reset(); } }