From a8c1c59a744adf7b1394f04517c34d8a26f046a1 Mon Sep 17 00:00:00 2001 From: Felipe Arce Date: Thu, 3 Sep 2026 22:38:15 +0000 Subject: [PATCH 1/3] 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/3] 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(); } } From bf2aa3940cc23066da056f6d169cda8d3dbdff3d Mon Sep 17 00:00:00 2001 From: Felipe Arce Date: Fri, 13 Nov 2026 23:41:44 +0000 Subject: [PATCH 3/3] Fail fast after a fatal Python exception instead of reusing freed objects handle_py_exception() released the slave's Python objects (pClass_, pInstance_, pMessages_) with Py_XDECREF but kept the pointers, and the FMI function then returned fmi2Fatal. Every later call on that component dereferenced or released freed memory: fmi2SetReal, fmi2DoStep or fmi2SetupExperiment called a method on the freed slave, fmi2Reset released it a second time before re-creating it, and fmi2FreeInstance (whose destructor runs cleanPyObject() again) released all three a second time. A host that keeps going after a slave raises -- one that re-initialises or replaces the instance, or simply terminates and frees it -- died with SIGSEGV/SIGBUS; under PYTHONMALLOC=debug the crash is deterministic. initialize() had the same flaw on its own failure path: it released pInstance_ and pMessages_ before re-creating them, so a slave constructor that raised during fmi2Reset left pMessages_ dangling for the cleanup that follows. Mark the instance failed on the first Python exception and release the objects with Py_CLEAR. Every later operation throws fatal_error, so the FMI function returns fmi2Fatal without touching Python, and the destructor only releases what is still held, so fmi2FreeInstance is safe. Also fetch the pending exception before the cleanup runs Python code, survive a failing repr() in the error report, report a failed _get_log_queue() instead of ignoring it, and fail (instead of carrying on with a NULL class) when no Python error is set, which happens when findClass() cannot open the slave module. --- src/pythonfmu/PySlaveInstance.cpp | 104 ++++++++++++++++++++---------- 1 file changed, 70 insertions(+), 34 deletions(-) diff --git a/src/pythonfmu/PySlaveInstance.cpp b/src/pythonfmu/PySlaveInstance.cpp index 4965f8e..a47c5c7 100644 --- a/src/pythonfmu/PySlaveInstance.cpp +++ b/src/pythonfmu/PySlaveInstance.cpp @@ -204,8 +204,8 @@ class PySlaveInstance : public SlaveInstance void initialize(PyGILState_STATE gilState) { - Py_XDECREF(pInstance_); - Py_XDECREF(pMessages_); + Py_CLEAR(pInstance_); + Py_CLEAR(pMessages_); PyObject* args = PyTuple_New(0); PyObject* kwargs = Py_BuildValue("{ss,ss,sn,si}", @@ -220,10 +220,14 @@ class PySlaveInstance : public SlaveInstance handle_py_exception("[initialize] PyObject_Call", gilState); } pMessages_ = PyObject_CallMethod(pInstance_, "_get_log_queue", nullptr); + if (pMessages_ == nullptr) { + handle_py_exception("[initialize] _get_log_queue", gilState); + } } void SetupExperiment(double startTime, std::optional stop, std::optional tolerance) override { + ensureUsable("[setupExperiment]"); py_safe_run([this, startTime, stop, tolerance](PyGILState_STATE gilState) { PyObject* pyStop = stop ? Py_BuildValue("d", *stop) : (Py_INCREF(Py_None), Py_None); PyObject* pyTol = tolerance ? Py_BuildValue("d", *tolerance) : (Py_INCREF(Py_None), Py_None); @@ -243,6 +247,7 @@ class PySlaveInstance : public SlaveInstance void EnterInitializationMode() override { + ensureUsable("[enterInitializationMode]"); py_safe_run([this](PyGILState_STATE gilState) { auto f = PyObject_CallMethod(pInstance_, "enter_initialization_mode", nullptr); if (f == nullptr) { @@ -255,6 +260,7 @@ class PySlaveInstance : public SlaveInstance void ExitInitializationMode() override { + ensureUsable("[exitInitializationMode]"); py_safe_run([this](PyGILState_STATE gilState) { auto f = PyObject_CallMethod(pInstance_, "exit_initialization_mode", nullptr); if (f == nullptr) { @@ -267,6 +273,7 @@ class PySlaveInstance : public SlaveInstance bool Step(double currentTime, double stepSize) override { + ensureUsable("[doStep]"); bool status; py_safe_run([this, &status, currentTime, stepSize](PyGILState_STATE gilState) { auto f = PyObject_CallMethod(pInstance_, "do_step", "(dd)", currentTime, stepSize); @@ -283,6 +290,7 @@ class PySlaveInstance : public SlaveInstance void Reset() override { + ensureUsable("[reset]"); py_safe_run([this](PyGILState_STATE gilState) { initialize(gilState); }); @@ -290,6 +298,7 @@ class PySlaveInstance : public SlaveInstance void Terminate() override { + ensureUsable("[terminate]"); py_safe_run([this](PyGILState_STATE gilState) { auto f = PyObject_CallMethod(pInstance_, "terminate", nullptr); if (f == nullptr) { @@ -302,6 +311,7 @@ class PySlaveInstance : public SlaveInstance void SetReal(const fmi2ValueReference* vr, std::size_t nvr, const fmi2Real* values) override { + ensureUsable("[setReal]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); PyObject* refs = PyList_New(nvr); @@ -323,6 +333,7 @@ class PySlaveInstance : public SlaveInstance void SetInteger(const fmi2ValueReference* vr, std::size_t nvr, const fmi2Integer* values) override { + ensureUsable("[setInteger]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); PyObject* refs = PyList_New(nvr); @@ -344,6 +355,7 @@ class PySlaveInstance : public SlaveInstance void SetBoolean(const fmi2ValueReference* vr, std::size_t nvr, const fmi2Boolean* values) override { + ensureUsable("[setBoolean]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); PyObject* refs = PyList_New(nvr); @@ -365,6 +377,7 @@ class PySlaveInstance : public SlaveInstance void SetString(const fmi2ValueReference* vr, std::size_t nvr, fmi2String const* values) override { + ensureUsable("[setString]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); PyObject* refs = PyList_New(nvr); @@ -386,6 +399,7 @@ class PySlaveInstance : public SlaveInstance void GetReal(const fmi2ValueReference* vr, std::size_t nvr, fmi2Real* values) const override { + ensureUsable("[getReal]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); for (int i = 0; i < nvr; i++) { @@ -409,6 +423,7 @@ class PySlaveInstance : public SlaveInstance void GetInteger(const fmi2ValueReference* vr, std::size_t nvr, fmi2Integer* values) const override { + ensureUsable("[getInteger]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); for (int i = 0; i < nvr; i++) { @@ -431,6 +446,7 @@ class PySlaveInstance : public SlaveInstance void GetBoolean(const fmi2ValueReference* vr, std::size_t nvr, fmi2Boolean* values) const override { + ensureUsable("[getBoolean]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { PyObject* vrs = PyList_New(nvr); for (int i = 0; i < nvr; i++) { @@ -453,6 +469,7 @@ class PySlaveInstance : public SlaveInstance void GetString(const fmi2ValueReference* vr, std::size_t nvr, fmi2String* values) const override { + ensureUsable("[getString]"); py_safe_run([this, &vr, nvr, &values](PyGILState_STATE gilState) { clearStrBuffer(); PyObject* vrs = PyList_New(nvr); @@ -477,6 +494,7 @@ class PySlaveInstance : public SlaveInstance void GetFMUstate(fmi2FMUstate& state) override { + ensureUsable("[_get_fmu_state]"); py_safe_run([this, &state](PyGILState_STATE gilState) { auto f = PyObject_CallMethod(pInstance_, "_get_fmu_state", nullptr); if (f == nullptr) { @@ -489,6 +507,7 @@ class PySlaveInstance : public SlaveInstance void SetFMUstate(const fmi2FMUstate& state) override { + ensureUsable("[_set_fmu_state]"); py_safe_run([this, &state](PyGILState_STATE gilState) { auto pyState = reinterpret_cast(state); auto f = PyObject_CallMethod(pInstance_, "_set_fmu_state", "(O)", pyState); @@ -501,6 +520,7 @@ class PySlaveInstance : public SlaveInstance void FreeFMUstate(fmi2FMUstate& state) override { + ensureUsable("[FreeFMUstate]"); py_safe_run([this, &state](PyGILState_STATE gilState) { auto f = reinterpret_cast(state); Py_XDECREF(f); @@ -509,6 +529,7 @@ class PySlaveInstance : public SlaveInstance size_t SerializedFMUstateSize(const fmi2FMUstate& state) override { + ensureUsable("[SerializedFMUstateSize]"); size_t size; py_safe_run([this, &state, &size](PyGILState_STATE gilState) { auto pyState = reinterpret_cast(state); @@ -525,6 +546,7 @@ class PySlaveInstance : public SlaveInstance void SerializeFMUstate(const fmi2FMUstate& state, fmi2Byte* bytes, size_t size) override { + ensureUsable("[SerializeFMUstate]"); py_safe_run([this, &state, &bytes, size](PyGILState_STATE gilState) { auto pyState = reinterpret_cast(state); PyObject* pyStateBytes = PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState); @@ -545,6 +567,7 @@ class PySlaveInstance : public SlaveInstance void DeSerializeFMUstate(const fmi2Byte bytes[], size_t size, fmi2FMUstate& state) override { + ensureUsable("[DeSerializeFMUstate]"); py_safe_run([this, &bytes, size, &state](PyGILState_STATE gilState) { PyObject* pyStateBytes = PyBytes_FromStringAndSize(bytes, size); if (pyStateBytes == nullptr) { @@ -570,9 +593,15 @@ class PySlaveInstance : public SlaveInstance private: fmu_data data_; - PyObject* pClass_; - PyObject* pInstance_{}; - PyObject* pMessages_{}; + // Owned references, released with Py_CLEAR so no path can release one twice. + mutable PyObject* pClass_{}; + mutable PyObject* pInstance_{}; + mutable PyObject* pMessages_{}; + + // Set by the first Python exception, whose handler releases the slave object. + // Every later operation fails fast (fmi2Fatal) without touching Python, and the + // destructor (fmi2FreeInstance) only releases what is still held. + mutable bool failed_{false}; mutable std::vector strBuffer; mutable std::vector logStrBuffer; @@ -617,44 +646,51 @@ class PySlaveInstance : public SlaveInstance clearLogBuffer(); clearLogStrBuffer(); clearStrBuffer(); - Py_XDECREF(pClass_); - Py_XDECREF(pInstance_); - Py_XDECREF(pMessages_); + Py_CLEAR(pClass_); + Py_CLEAR(pInstance_); + Py_CLEAR(pMessages_); } - - void handle_py_exception(const std::string& what, PyGILState_STATE gilState) const + void ensureUsable(const char* what) const { - const auto err = PyErr_Occurred(); - if (err != nullptr) { - cleanPyObject(); + if (failed_) { + throw fatal_error(std::string(what) + + " refused: an earlier Python exception was fatal and released this " + "instance's slave object; only fmi2FreeInstance is valid now"); + } + } - PyObject *pExcType, *pExcValue, *pExcTraceback; - PyErr_Fetch(&pExcType, &pExcValue, &pExcTraceback); + // Never returns: marks this instance failed, releases its Python objects and the + // GIL taken by py_safe_run, and throws fatal_error (reported as fmi2Fatal). + [[noreturn]] void handle_py_exception(const std::string& what, PyGILState_STATE gilState) const + { + failed_ = true; - std::ostringstream oss; - oss << "Fatal py exception encountered: "; - oss << what << "\n"; - if (pExcValue != nullptr) { - PyObject* pRepr = PyObject_Repr(pExcValue); - PyObject* pyStr = PyUnicode_AsEncodedString(pRepr, "utf-8", nullptr); - oss << PyBytes_AsString(pyStr); - Py_DECREF(pyStr); - Py_DECREF(pRepr); - } else { - oss << "unknown error"; - } + std::ostringstream oss; + oss << "Fatal py exception encountered: "; + oss << what << "\n"; - PyErr_Clear(); + // Take the pending exception first, so the cleanup below never runs Python + // code (log flushing, __del__) with an exception already set. + PyObject *pExcType, *pExcValue, *pExcTraceback; + PyErr_Fetch(&pExcType, &pExcValue, &pExcTraceback); + PyObject* pRepr = pExcValue != nullptr ? PyObject_Repr(pExcValue) : nullptr; + PyObject* pyStr = pRepr != nullptr ? PyUnicode_AsEncodedString(pRepr, "utf-8", nullptr) : nullptr; + const char* text = pyStr != nullptr ? PyBytes_AsString(pyStr) : nullptr; + oss << (text != nullptr ? text : "unknown error"); + Py_XDECREF(pyStr); + Py_XDECREF(pRepr); + Py_XDECREF(pExcType); + Py_XDECREF(pExcValue); + Py_XDECREF(pExcTraceback); + PyErr_Clear(); // a failed repr/encode above - Py_XDECREF(pExcType); - Py_XDECREF(pExcValue); - Py_XDECREF(pExcTraceback); + cleanPyObject(); + PyErr_Clear(); // anything the cleanup raised - PyGILState_Release(gilState); + PyGILState_Release(gilState); - throw fatal_error(oss.str()); - } + throw fatal_error(oss.str()); } };