From a8c1c59a744adf7b1394f04517c34d8a26f046a1 Mon Sep 17 00:00:00 2001 From: Felipe Arce Date: Thu, 3 Sep 2026 22:38:15 +0000 Subject: [PATCH 1/2] Fix borrowed module dictionary reference handling PyModule_GetDict() returns a borrowed reference, but findClass() Py_DECREF'd it once per instantiation (and once more on the compile- error path). Each fmi2Instantiate of the same FMU therefore removed a reference the wrapper never owned; the module dictionary is eventually released while the module and its functions still point at it. Observable from the host: sys.getrefcount(module.__dict__) falls below len(gc.get_referrers(module.__dict__)) by one per instantiation. The compile-error path also called Py_Finalize() inside a shared library that does not own the interpreter (a Python host aborts with "_Py_GetConfig: ... thread state is NULL"), printed and cleared the exception so the caller could not report it, and Py_DECREF'd a NULL pCode. Stop decrementing the borrowed dictionary, leave interpreter finalization to the host, and leave the compile error set so that fmi2Instantiate fails with fmi2Fatal instead of crashing the process. Adds test_integration_reinstantiate_same_fmu. --- pythonfmu/tests/test_integration.py | 40 +++++++++++++++++++++++++++++ src/pythonfmu/PySlaveInstance.cpp | 8 +++--- 2 files changed, 43 insertions(+), 5 deletions(-) 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..6fc3f44 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; From 8c65171fbb3b871d38c91f30c52e41486458db9e Mon Sep 17 00:00:00 2001 From: Felipe Arce Date: Thu, 3 Sep 2026 22:38:47 +0000 Subject: [PATCH 2/2] Avoid PyState static destructor race at exit The namespace-scope std::shared_ptr was released twice at process exit: by its __cxa_atexit-registered destructor and again by finalizePythonInterpreter() from the library unload hook, which _dl_fini runs afterwards. libstdc++ does not null the control-block pointer on destruction, so the second release touched freed memory. Hosts saw an intermittent "corrupted double-linked list" abort on exit; AddressSanitizer reports the heap-use-after-free on every exit, including a plain fmpy.simulate_fmu() run. Keep the holder heap-allocated for the life of the process (no static destructor) and reset it under pyStateMutex from the unload hook, which stays idempotent. Also hand the shared PyState to the slave *before* constructing it: the assignment after std::make_unique was a dead store since #237 (data is passed by value), so slaves never actually co-owned PyState. They now do, as #211/#213 intended, so the interpreter cannot be finalized under a live slave. --- src/pythonfmu/PySlaveInstance.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/pythonfmu/PySlaveInstance.cpp b/src/pythonfmu/PySlaveInstance.cpp index 6fc3f44..4965f8e 100644 --- a/src/pythonfmu/PySlaveInstance.cpp +++ b/src/pythonfmu/PySlaveInstance.cpp @@ -662,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; } } @@ -687,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(); } }