Repository navigation
Conversation
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.
The namespace-scope std::shared_ptr<PyState> 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 NTNU-IHB#237 (data is passed by value), so slaves never actually co-owned PyState. They now do, as NTNU-IHB#211/NTNU-IHB#213 intended, so the interpreter cannot be finalized under a live slave.
…ects 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #259 (its two commits are included here; review only the last commit,
bf2aa39).handle_py_exception()releases the slave's Python objects (pClass_,pInstance_,pMessages_) but keeps the pointers, and the FMI function returnsfmi2Fatal. Any later call on that component uses or re-releases freed memory:fmi2SetReal/fmi2DoStep/fmi2SetupExperimentcall a method on the freed slave,fmi2Resetreleases it again before re-creating it, andfmi2FreeInstance(the destructor runscleanPyObject()again) releases all three a second time. A host that keeps going after a slave raises — re-initialising, replacing, or just terminate-then-free — dies with SIGSEGV/SIGBUS; deterministic underPYTHONMALLOC=debug.initialize()has the same flaw when the slave constructor raises duringfmi2Reset.This commit marks the instance failed on the first Python exception and releases the objects with
Py_CLEAR. Every later operation throwsfatal_errorbefore touching Python (so the FMI call returnsfmi2Fatal), and the destructor only releases what is still held, sofmi2FreeInstanceon a failed instance is safe. It also fetches the pending exception before cleanup runs Python code, survives a failingrepr()in the error report, reports a failed_get_log_queue(), and fails cleanly whenfindClass()cannot open the slave module.Verified with a probe FMU whose
do_stepraises, followed by 24 forbidden calls and both endings (free; leak to process exit) underPYTHONMALLOC=debug: unpatched segfaults (-11), patched exits 0. Running in production in our simulator fleet.