Fix borrowed module dictionary reference and PyState exit-time use-after-free in the export wrapper

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

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> 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;
     }
 }
@@ -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();
 }
 }