Skip to content

Fail fast after a fatal Python exception instead of reusing freed slave objects #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
astherath wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fail fast after a fatal Python exception instead of reusing freed slave objects #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8c1c59
Fix borrowed module dictionary reference handling
astherath Sep 3, 2026
8c65171
Avoid PyState static destructor race at exit
astherath Sep 3, 2026
bf2aa39
Fail fast after a fatal Python exception instead of reusing freed obj…
astherath Nov 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions pythonfmu/tests/test_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
132 changes: 86 additions & 46 deletions src/pythonfmu/PySlaveInstance.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -206,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}",
Expand All @@ -222,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<double> stop, std::optional<double> 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);
Expand All @@ -245,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) {
Expand All @@ -257,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) {
Expand All @@ -269,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);
Expand All @@ -285,13 +290,15 @@ class PySlaveInstance : public SlaveInstance

void Reset() override
{
ensureUsable("[reset]");
py_safe_run([this](PyGILState_STATE gilState) {
initialize(gilState);
});
}

void Terminate() override
{
ensureUsable("[terminate]");
py_safe_run([this](PyGILState_STATE gilState) {
auto f = PyObject_CallMethod(pInstance_, "terminate", nullptr);
if (f == nullptr) {
Expand All @@ -304,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);
Expand All @@ -325,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);
Expand All @@ -346,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);
Expand All @@ -367,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);
Expand All @@ -388,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++) {
Expand All @@ -411,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++) {
Expand All @@ -433,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++) {
Expand All @@ -455,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);
Expand All @@ -479,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) {
Expand All @@ -491,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<PyObject*>(state);
auto f = PyObject_CallMethod(pInstance_, "_set_fmu_state", "(O)", pyState);
Expand All @@ -503,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<PyObject*>(state);
Py_XDECREF(f);
Expand All @@ -511,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<PyObject*>(state);
Expand All @@ -527,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<PyObject*>(state);
PyObject* pyStateBytes = PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState);
Expand All @@ -547,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) {
Expand All @@ -572,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<PyObject*> strBuffer;
mutable std::vector<PyObject*> logStrBuffer;
Expand Down Expand Up @@ -619,66 +646,78 @@ 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());
}
};

namespace
{

std::mutex pyStateMutex{};
std::shared_ptr<PyState> pyState{};
// Intentionally heap-allocated and never deleted. A namespace-scope
// std::shared_ptr<PyState> 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>* pyState = new std::shared_ptr<PyState>();

} // namespace

std::unique_ptr<SlaveInstance> pythonfmu::createInstance(fmu_data data)
{
{
auto const ensurePyStateAlive = [&]() {
auto const ensurePyStateAlive = [&]() -> std::shared_ptr<PyState> {
auto const lock = std::lock_guard{pyStateMutex};
if (nullptr == pyState) pyState = std::make_shared<PyState>();
if (nullptr == *pyState) *pyState = std::make_shared<PyState>();
return *pyState;
};

ensurePyStateAlive();
data.pyState = ensurePyStateAlive();
auto c = std::make_unique<PySlaveInstance>(data);
data.pyState = pyState;
return c;
}
}
Expand All @@ -689,10 +728,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();
}
}

Expand Down