From 9e511b46b9a0ed35225a90b585533da04fb2fa8a Mon Sep 17 00:00:00 2001 From: Stephen Smith Date: Thu, 12 Nov 2026 19:20:58 +0000 Subject: [PATCH] Fix wrapper memory-safety bugs and builder option/terminals bugs C++ wrapper: - RAII GIL guard and PyObject refs so every exit path releases the GIL - Fix double Py_DECREF after a Python exception (use-after-free on free) - Report Python errors with full traceback; stop PyErr_Print swallowing them - Bounds-check lists returned by getters instead of reading past the end - Always write doStep outputs; honour eventHandlingNeeded/earlyReturn - Release overwritten FMU states and SetFMUState results - Fix log buffer leaks, Component leak on failed instantiation, NULL log callback crash, and decode file:// resource URIs fully - Read the slave class name from slavemodule.txt (regex as fallback) Python: - to_xml used a stale loop value for CoSimulation/ModelExchange options - --terminals packed the wrong file into terminalsAndIcons/ - np.reshape(newshape=...) fails on NumPy >= 2.5 - Builder finds the FMU class by inspection and records it in the FMU Tests: builder regressions and a ctypes driver exercising the wrapper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X8Wmw797DJpgrMFmT6n8CG --- pythonfmu3/builder.py | 72 +- pythonfmu3/fmi3slave.py | 42 +- .../src/cppfmu/cppfmu_common.hpp | 3 + .../src/cppfmu/fmi_functions.cpp | 10 +- .../src/pythonfmu/PySlaveInstance.cpp | 1110 ++++++++--------- .../src/pythonfmu/PySlaveInstance.hpp | 27 +- pythonfmu3/tests/fmi3_ctypes_driver.py | 137 ++ pythonfmu3/tests/test_builder.py | 48 +- pythonfmu3/tests/test_variables.py | 2 +- pythonfmu3/tests/test_wrapper_ctypes.py | 61 + 10 files changed, 895 insertions(+), 617 deletions(-) create mode 100644 pythonfmu3/tests/fmi3_ctypes_driver.py create mode 100644 pythonfmu3/tests/test_wrapper_ctypes.py diff --git a/pythonfmu3/builder.py b/pythonfmu3/builder.py index 58be085..602dadb 100644 --- a/pythonfmu3/builder.py +++ b/pythonfmu3/builder.py @@ -1,6 +1,6 @@ """Python FMU builder""" import argparse -import importlib +import importlib.util import itertools import logging import re @@ -21,13 +21,49 @@ logger = logging.getLogger(__name__) +_SLAVE_CLASS_RE = re.compile(r'^class (\w+)\(([^)]*\bFmi3Slave(?:Base)?\b[^)]*)\)\s*:', re.MULTILINE) + + def get_class_name(file_name: Path) -> str: - with open(str(file_name), 'r') as file: - data = file.read() - return re.search(r'class (\w+)\(([^)]*\bFmi3Slave(?:Base)?\b[^)]*)\)\s*:', data).group(1) + """Find the name of the first class in *file_name* that textually extends Fmi3Slave/Fmi3SlaveBase.""" + data = Path(file_name).read_text(encoding="utf-8") + match = _SLAVE_CLASS_RE.search(data) + if match is None: + raise ValueError(f"Unable to find a class extending Fmi3Slave or Fmi3SlaveBase in {file_name!s}") + return match.group(1) + + +def _find_slave_class(module, filepath: Path) -> type: + """Locate the FMU class in an imported module. + + Classes defined in the module that inherit from Fmi3SlaveBase are candidates. If the + regex-based lookup (also used by the native wrapper as a fallback) matches one of them + it wins, so behaviour stays identical for existing scripts; otherwise a single + candidate is accepted (this handles multi-line base lists and indirect inheritance). + """ + candidates = { + name: obj for name, obj in vars(module).items() + if isinstance(obj, type) + and obj.__module__ == module.__name__ + and any(base.__name__ == Fmi3SlaveBase.__name__ for base in obj.__mro__) + } + try: + preferred = get_class_name(filepath) + except ValueError: + preferred = None + if preferred in candidates: + return candidates[preferred] + if len(candidates) == 1: + return next(iter(candidates.values())) + if not candidates: + raise ValueError(f"No class inheriting from {Fmi3SlaveBase.__qualname__} found in {filepath!s}") + raise ValueError( + f"Several FMU classes found in {filepath!s} ({', '.join(sorted(candidates))}); " + "keep a single Fmi3Slave subclass per script." + ) -def get_model_description(filepath: Path, module_name: str) -> Tuple[str, Element]: +def get_model_description(filepath: Path, module_name: str) -> Tuple[str, Element, str]: """Extract the FMU model description as XML. Args: @@ -35,7 +71,7 @@ def get_model_description(filepath: Path, module_name: str) -> Tuple[str, Elemen module_name (str) : python module to load Returns: - Tuple[str, xml.etree.TreeElement.Element] : FMU model name, model description + Tuple[str, xml.etree.TreeElement.Element, str] : FMU model name, model description, class name """ # Add current folder to handle local dependencies sys.path.insert(0, str(filepath.parent)) @@ -45,8 +81,9 @@ def get_model_description(filepath: Path, module_name: str) -> Tuple[str, Elemen fmu_interface = importlib.util.module_from_spec(spec) spec.loader.exec_module(fmu_interface) # Instantiate the interface - class_name = get_class_name(filepath) - instance = getattr(fmu_interface, class_name)(instance_name="dummyInstance", resources=str(filepath.parent)) + slave_class = _find_slave_class(fmu_interface, filepath) + class_name = slave_class.__name__ + instance = slave_class(instance_name="dummyInstance", resources=str(filepath.parent)) finally: sys.path.remove(str(filepath.parent)) # remove inserted temporary path @@ -55,7 +92,7 @@ def get_model_description(filepath: Path, module_name: str) -> Tuple[str, Elemen f"The provided class '{class_name}' does not inherit from {Fmi3SlaveBase.__qualname__}" ) # Produce the xml - return instance.modelName, instance.to_xml() + return instance.modelName, instance.to_xml(), class_name class FmuBuilder: @@ -64,7 +101,7 @@ class FmuBuilder: def build_FMU( script_file: FilePath, dest: FilePath = ".", - project_files: Iterable[FilePath] = set(), + project_files: Iterable[FilePath] = (), documentation_folder: Optional[FilePath] = None, terminals : Optional[FilePath] = None, **options, @@ -130,7 +167,7 @@ def build_FMU( else: shutil.copy2(file_, temp_dir) - model_identifier, xml = get_model_description( + model_identifier, xml, class_name = get_model_description( temp_dir.absolute() / script_file.name, module_name ) @@ -163,8 +200,11 @@ def build_FMU( relative_f = f.relative_to(temp_dir) zip_fmu.write(f, arcname=(resource / relative_f)) - # Add information for the Python loader - zip_fmu.writestr(str(resource.joinpath("slavemodule.txt")), module_name) + # Add information for the Python loader: line 1 is the module, line 2 the class. + # Older wrappers only read the first line, so this stays backward compatible. + zip_fmu.writestr( + str(resource.joinpath("slavemodule.txt")), f"{module_name}\n{class_name}\n" + ) # Add FMI API wrapping Python class source sources = Path("sources") @@ -174,7 +214,7 @@ def build_FMU( ): relative_f = f.relative_to(src) if "build" in relative_f.parts: - continue; + continue zip_fmu.write(f, arcname=(sources / relative_f)) # Add FMI API wrapping Python class library @@ -203,9 +243,7 @@ def build_FMU( if terminals is not None: terminalsFolder = Path("terminalsAndIcons") - relative_f = terminals.relative_to(terminals.parent) - zip_fmu.write(f, arcname=(terminalsFolder / relative_f)) - + zip_fmu.write(terminals, arcname=(terminalsFolder / terminals.name)) # Add the model description xml_str = parseString(tostring(xml, "UTF-8")) diff --git a/pythonfmu3/fmi3slave.py b/pythonfmu3/fmi3slave.py index 2a95dc1..33c78c3 100644 --- a/pythonfmu3/fmi3slave.py +++ b/pythonfmu3/fmi3slave.py @@ -76,7 +76,7 @@ def __init__(self, **kwargs): self.type_definitions: Dict[str, VariableType] = {} self.units: Dict[str, Unit] = {} - def to_xml(self, model_options: Dict[str, str] = dict()) -> Element: + def to_xml(self, model_options: Optional[Dict[str, Any]] = None) -> Element: """Build the XML representation of the model. Args: @@ -110,20 +110,24 @@ def to_xml(self, model_options: Dict[str, str] = dict()) -> Element: root = Element("fmiModelDescription", attrib) + model_options = model_options or {} + + def _resolve(option: ModelOptions) -> str: + return str(model_options.get(option.name, option.value)).lower() + options = dict() for option in FMI3_MODEL_OPTIONS_COMMON: - value = model_options.get(option.name, option.value) - options[option.name] = str(value).lower() + options[option.name] = _resolve(option) options["modelIdentifier"] = self.modelName options_cs = options.copy() for option in FMI3_MODEL_OPTIONS_COSIM: - options_cs[option.name] = str(value).lower() + options_cs[option.name] = _resolve(option) options_me = options.copy() for option in FMI3_MODEL_OPTIONS_MX: - options_me[option.name] = str(value).lower() + options_me[option.name] = _resolve(option) # check if we have cosim mixin or model exchange mixin if isinstance(self, ModelExchange): @@ -175,9 +179,7 @@ def to_xml(self, model_options: Dict[str, str] = dict()) -> Element: filter(lambda v: v.causality == Fmi3Causality.output, self.vars.values()) ) - continuous_state_derivatives = list( - filter(lambda v: v.variability == Fmi3Variability.continuous and (isinstance(v, Float64) and v.derivative is not None), self.vars.values()) - ) + continuous_state_derivatives = self._continuous_state_derivatives() allowed_variability = [None, Fmi3Initial.approx, Fmi3Initial.calculated] initial_unknown = list( @@ -200,6 +202,15 @@ def to_xml(self, model_options: Dict[str, str] = dict()) -> Element: return root + def _continuous_state_derivatives(self) -> List[Float64]: + """Return the registered derivative variables (one per continuous state), in registration order.""" + return [ + v for v in self.vars.values() + if isinstance(v, Float64) + and v.variability == Fmi3Variability.continuous + and v.derivative is not None + ] + def __apply_start_value(self, var: ModelVariable): vrs = [var.value_reference] if isinstance(var, Int32): @@ -243,7 +254,7 @@ def register_variable(self, var: ModelVariable, nested: bool = True, var_type: A if var.setter is None and hasattr(owner, var.local_name) and var.variability != Fmi3Variability.constant: if hasattr(var, "dimensions") and len(var.dimensions) > 0: import numpy as np - var.setter = lambda v: setattr(owner, var.local_name, np.reshape(v, newshape=getattr(owner, var.local_name).shape)) + var.setter = lambda v: setattr(owner, var.local_name, np.reshape(v, getattr(owner, var.local_name).shape)) else: var.setter = lambda v: setattr(owner, var.local_name, v) @@ -476,9 +487,7 @@ def get_number_of_event_indicators(self) -> int: def set_continuous_states(self, values: List[float]): offset = 0 - continuous_state_derivatives = list( - filter(lambda v: v.variability == Fmi3Variability.continuous and (isinstance(v, Float64) and v.derivative is not None), self.vars.values()) - ) + continuous_state_derivatives = self._continuous_state_derivatives() vrs = [v.derivative for v in continuous_state_derivatives] @@ -492,10 +501,7 @@ def set_continuous_states(self, values: List[float]): offset += size def get_continuous_states(self) -> List[float]: - offset = 0 - continuous_state_derivatives = list( - filter(lambda v: v.variability == Fmi3Variability.continuous and (isinstance(v, Float64) and v.derivative is not None), self.vars.values()) - ) + continuous_state_derivatives = self._continuous_state_derivatives() vrs = [v.derivative for v in continuous_state_derivatives] @@ -510,9 +516,7 @@ def get_continuous_states(self) -> List[float]: return refs def get_number_of_continuous_states(self) -> int: - continuous_state_derivatives = list( - filter(lambda v: v.variability == Fmi3Variability.continuous and (isinstance(v, Float64) and v.derivative is not None), self.vars.values()) - ) + continuous_state_derivatives = self._continuous_state_derivatives() return len(continuous_state_derivatives) def set_time(self, time: float): diff --git a/pythonfmu3/pythonfmu-export/src/cppfmu/cppfmu_common.hpp b/pythonfmu3/pythonfmu-export/src/cppfmu/cppfmu_common.hpp index 4855b3f..7583b64 100644 --- a/pythonfmu3/pythonfmu-export/src/cppfmu/cppfmu_common.hpp +++ b/pythonfmu3/pythonfmu-export/src/cppfmu/cppfmu_common.hpp @@ -121,6 +121,9 @@ class Logger FMIString message, Args&&... args) CPPFMU_NOEXCEPT { + if (m_fmiLogger == nullptr) { + return; // FMI 3.0 allows instantiating without a logger callback + } if (m_settings->loggedCategories.empty() || detail::CanFind(m_settings->loggedCategories, category)) { m_fmiLogger( diff --git a/pythonfmu3/pythonfmu-export/src/cppfmu/fmi_functions.cpp b/pythonfmu3/pythonfmu-export/src/cppfmu/fmi_functions.cpp index 6cd61d4..293ad84 100644 --- a/pythonfmu3/pythonfmu-export/src/cppfmu/fmi_functions.cpp +++ b/pythonfmu3/pythonfmu-export/src/cppfmu/fmi_functions.cpp @@ -95,7 +95,8 @@ fmi3Instance fmi3InstantiateCoSimulation( fmi3IntermediateUpdateCallback intermediateUpdate ) { try { - auto component = new Component( + // Owned until fully constructed, so a failing instantiation does not leak it. + auto component = std::make_unique( environment, logMessage, loggingOn); @@ -108,7 +109,7 @@ fmi3Instance fmi3InstantiateCoSimulation( visible, cppfmu::FMIFalse, component->logger); - return component; + return component.release(); } catch (const cppfmu::FatalError& e) { if (logMessage) { @@ -134,7 +135,8 @@ fmi3Instance fmi3InstantiateModelExchange( fmi3LogMessageCallback logMessage) { try { - auto component = new Component( + // Owned until fully constructed, so a failing instantiation does not leak it. + auto component = std::make_unique( environment, logMessage, loggingOn); @@ -147,7 +149,7 @@ fmi3Instance fmi3InstantiateModelExchange( visible, cppfmu::FMIFalse, component->logger); - return component; + return component.release(); } catch (const cppfmu::FatalError& e) { if (logMessage) { diff --git a/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.cpp b/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.cpp index f4d71ad..b7179e2 100644 --- a/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.cpp +++ b/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.cpp @@ -6,32 +6,101 @@ #include "cppfmu/cppfmu_cs.hpp" +#include +#include #include -#include #include #include #include +#include #include namespace pythonfmu { -inline std::string getline(const std::string& fileName) +namespace { - std::string line; - std::ifstream infile(fileName); - std::getline(infile, line); - return line; + +// Holds the GIL for the lifetime of the object. Releasing in the destructor guarantees +// the GIL is given back on every exit path, including C++ exceptions. +class GilGuard +{ +public: + GilGuard() + : state_(PyGILState_Ensure()) + { } + ~GilGuard() { PyGILState_Release(state_); } + GilGuard(const GilGuard&) = delete; + GilGuard& operator=(const GilGuard&) = delete; + +private: + PyGILState_STATE state_; +}; + +// Owning reference to a PyObject (Py_XDECREF on destruction). +// Must only be destroyed while the GIL is held; declare it after the GilGuard. +class PyRef +{ +public: + explicit PyRef(PyObject* obj = nullptr) + : obj_(obj) + { } + ~PyRef() { Py_XDECREF(obj_); } + PyRef(const PyRef&) = delete; + PyRef& operator=(const PyRef&) = delete; + PyRef(PyRef&& other) noexcept + : obj_(other.release()) + { } + + PyObject* get() const { return obj_; } + PyObject* release() + { + PyObject* tmp = obj_; + obj_ = nullptr; + return tmp; + } + explicit operator bool() const { return obj_ != nullptr; } + +private: + PyObject* obj_; +}; + +template +auto py_safe_run(F&& f) -> decltype(f()) +{ + GilGuard gil; + return f(); } -inline std::string findClassName(const std::string& fileName) +// Reads the first two lines of resources/slavemodule.txt: the module name and +// (for FMUs built by newer versions of PythonFMU3) the name of the slave class. +std::pair readSlaveModuleInfo(const std::string& fileName) +{ + std::ifstream infile(fileName); + if (!infile) { + throw cppfmu::FatalError(("Unable to open " + fileName).c_str()); + } + std::string moduleName, className; + std::getline(infile, moduleName); + std::getline(infile, className); + auto trim = [](std::string& s) { + const auto ws = " \t\r\n"; + s.erase(s.find_last_not_of(ws) + 1); + s.erase(0, s.find_first_not_of(ws)); + }; + trim(moduleName); + trim(className); + return {moduleName, className}; +} + +// Fallback for FMUs built by older versions that do not record the class name. +std::string findClassName(const std::string& fileName) { std::string line; std::ifstream infile(fileName); - std::string regexStr(R"(^class (\w+)\(([^)]*\bFmi3Slave(?:Base)?\b[^)]*)\)\s*:)"); - while (getline(infile, line)) { + const std::regex re(R"(^class (\w+)\(([^)]*\bFmi3Slave(?:Base)?\b[^)]*)\)\s*:)"); + while (std::getline(infile, line)) { std::smatch m; - std::regex re(regexStr); if (std::regex_search(line, m, re)) { return m[1]; } @@ -39,63 +108,137 @@ inline std::string findClassName(const std::string& fileName) return ""; } -inline void py_safe_run(const std::function& f) +// Returns nullptr (with a Python error set) on failure. +PyObject* makeVrList(const cppfmu::FMIValueReference* vr, std::size_t nvr) { - PyGILState_STATE gil_state = PyGILState_Ensure(); - f(gil_state); - PyGILState_Release(gil_state); + PyRef list(PyList_New(static_cast(nvr))); + if (!list) return nullptr; + for (std::size_t i = 0; i < nvr; i++) { + PyObject* item = PyLong_FromUnsignedLong(vr[i]); + if (item == nullptr) return nullptr; + PyList_SetItem(list.get(), static_cast(i), item); // steals item + } + return list.release(); } +template +PyObject* makeValueList(const T* values, std::size_t n, Convert convert) +{ + PyRef list(PyList_New(static_cast(n))); + if (!list) return nullptr; + for (std::size_t i = 0; i < n; i++) { + PyObject* item = convert(values[i]); + if (item == nullptr) return nullptr; + PyList_SetItem(list.get(), static_cast(i), item); // steals item + } + return list.release(); +} + +PyObject* makeString(cppfmu::FMIString s) +{ + if (s == nullptr) { + Py_RETURN_NONE; + } + return PyUnicode_FromString(s); +} + +std::string toUtf8(PyObject* str) +{ + PyRef bytes(str ? PyUnicode_AsEncodedString(str, "utf-8", "replace") : nullptr); + const char* data = bytes ? PyBytes_AsString(bytes.get()) : nullptr; + if (data == nullptr) { + PyErr_Clear(); + return {}; + } + return data; +} + +// Formats the pending Python exception, including its traceback, and clears it. +std::string fetchPythonError() +{ + PyObject *type = nullptr, *value = nullptr, *traceback = nullptr; + PyErr_Fetch(&type, &value, &traceback); + if (type == nullptr) { + return "unknown error (no Python exception set)"; + } + PyErr_NormalizeException(&type, &value, &traceback); + PyRef pType(type), pValue(value), pTraceback(traceback); + + std::string result; + PyRef tbModule(PyImport_ImportModule("traceback")); + if (tbModule) { + PyRef lines(PyObject_CallMethod(tbModule.get(), "format_exception", "OOO", + pType.get(), pValue ? pValue.get() : Py_None, pTraceback ? pTraceback.get() : Py_None)); + if (lines) { + PyRef sep(PyUnicode_FromString("")); + PyRef joined(sep ? PyUnicode_Join(sep.get(), lines.get()) : nullptr); + if (joined) result = toUtf8(joined.get()); + } + } + if (result.empty()) { + PyRef repr(PyObject_Repr(pValue ? pValue.get() : pType.get())); + result = toUtf8(repr.get()); + if (result.empty()) result = "unknown error"; + } + PyErr_Clear(); // in case formatting itself raised + return result; +} + + +// Reads `obj.name` as a bool. Returns `fallback` when the attribute is absent. +// Returns -1 (Python error set) on failure. +int readBoolAttr(PyObject* obj, const char* name, bool fallback) +{ + if (!PyObject_HasAttrString(obj, name)) return fallback ? 1 : 0; + PyRef attr(PyObject_GetAttrString(obj, name)); + if (!attr) return -1; + return PyObject_IsTrue(attr.get()); +} + +} // namespace + PySlaveInstance::PySlaveInstance(std::string instanceName, std::string resources, const cppfmu::Logger& logger, const bool visible, std::shared_ptr pyState) : pyState_{ std::move(pyState) } + , visible_(visible) , instanceName_(std::move(instanceName)) , resources_(std::move(resources)) , logger_(logger) - , visible_(visible) { - py_safe_run([this](PyGILState_STATE gilState) { - // Append resources path to python sys path - PyObject* sys_module = PyImport_ImportModule("sys"); - if (sys_module == nullptr) { - handle_py_exception("[ctor] PyImport_ImportModule", gilState); - } - PyObject* sys_path = PyObject_GetAttrString(sys_module, "path"); - Py_DECREF(sys_module); - if (sys_path == nullptr) { - handle_py_exception("[ctor] PyObject_GetAttrString", gilState); - } - int success = PyList_Insert(sys_path, 0, PyUnicode_FromString(resources_.c_str())); - Py_DECREF(sys_path); - if (success != 0) { - handle_py_exception("[ctor] PyList_Insert", gilState); - } + py_safe_run([this]() { + // The destructor does not run when a constructor throws, so release what was + // acquired so far before propagating the error. + try { + // Prepend resources path to python sys path + PyRef sysModule(PyImport_ImportModule("sys")); + if (!sysModule) handle_py_exception("[ctor] PyImport_ImportModule(sys)"); + PyRef sysPath(PyObject_GetAttrString(sysModule.get(), "path")); + if (!sysPath) handle_py_exception("[ctor] PyObject_GetAttrString(sys.path)"); + PyRef resourcesStr(PyUnicode_FromString(resources_.c_str())); + if (!resourcesStr || PyList_Insert(sysPath.get(), 0, resourcesStr.get()) != 0) { + handle_py_exception("[ctor] PyList_Insert"); + } - std::string moduleName = getline(resources_ + "/slavemodule.txt"); - PyObject* pModule = PyImport_ImportModule(moduleName.c_str()); - if (pModule == nullptr) { - PyErr_Print(); - handle_py_exception("[ctor] PyImport_ImportModule", gilState); - } + const auto info = readSlaveModuleInfo(resources_ + "/slavemodule.txt"); + const std::string& moduleName = info.first; + PyRef module(PyImport_ImportModule(moduleName.c_str())); + if (!module) handle_py_exception("[ctor] PyImport_ImportModule(" + moduleName + ")"); - std::string className = findClassName(resources_ + "/" + moduleName + ".py"); - if (className.empty()) { - cleanPyObject(); - throw cppfmu::FatalError("Unable to find class extending Fmi3SlaveBase!"); - } + std::string className = info.second; + if (className.empty()) { + className = findClassName(resources_ + "/" + moduleName + ".py"); + } + if (className.empty()) { + throw cppfmu::FatalError("Unable to find class extending Fmi3SlaveBase!"); + } - PyObject* pyClassName = Py_BuildValue("s", className.c_str()); - if (pyClassName == nullptr) { - handle_py_exception("[ctor] Py_BuildValue", gilState); - } + pClass_ = PyObject_GetAttrString(module.get(), className.c_str()); + if (pClass_ == nullptr) handle_py_exception("[ctor] PyObject_GetAttr(" + className + ")"); - pClass_ = PyObject_GetAttr(pModule, pyClassName); - Py_DECREF(pyClassName); - Py_DECREF(pModule); - if (pClass_ == nullptr) { - handle_py_exception("[ctor] PyObject_GetAttr", gilState); + initialize(); + } catch (...) { + cleanPyObject(); + throw; } - - initialize(gilState); }); } @@ -103,568 +246,423 @@ void PySlaveInstance::clearLogBuffer() const { clearLogStrBuffer(); - PyObject* debugField = Py_BuildValue("s", "debug"); - PyObject* msgField = Py_BuildValue("s", "msg"); - PyObject* categoryField = Py_BuildValue("s", "category"); - PyObject* statusField = Py_BuildValue("s", "status"); + if (pMessages_ == nullptr || !PyList_Check(pMessages_)) { + return; + } - if ( pMessages_ != NULL && PyList_Check(pMessages_) ) { - auto size = PyList_Size(pMessages_); - for (auto i = 0; i < size; i++) { - PyObject* msg = PyList_GetItem(pMessages_, i); + // Logging must never mask the original problem: keep any pending exception aside. + PyObject *errType, *errValue, *errTraceback; + PyErr_Fetch(&errType, &errValue, &errTraceback); - auto debugAttr = PyObject_GetAttr(msg, debugField); - auto msgAttr = PyObject_GetAttr(msg, msgField); - auto categoryAttr = PyObject_GetAttr(msg, categoryField); - auto statusAttr = PyObject_GetAttr(msg, statusField); + auto& logger = const_cast(logger_); + const auto size = PyList_Size(pMessages_); + for (Py_ssize_t i = 0; i < size; i++) { + PyObject* msg = PyList_GetItem(pMessages_, i); // borrowed - auto statusValue = static_cast(PyLong_AsLong(statusAttr)); + PyRef debugAttr(PyObject_GetAttrString(msg, "debug")); + PyRef msgAttr(PyObject_GetAttrString(msg, "msg")); + PyRef categoryAttr(PyObject_GetAttrString(msg, "category")); + PyRef statusAttr(PyObject_GetAttrString(msg, "status")); + if (!debugAttr || !msgAttr || !categoryAttr || !statusAttr) { + PyErr_Clear(); + continue; + } + + auto statusValue = static_cast(PyLong_AsLong(statusAttr.get())); - PyObject* msgValue = PyUnicode_AsEncodedString(msgAttr, "utf-8", nullptr); - char* msgStr = PyBytes_AsString(msgValue); - logStrBuffer.emplace_back(msgValue); + PyObject* msgValue = PyUnicode_AsEncodedString(msgAttr.get(), "utf-8", "replace"); + if (msgValue == nullptr) { + PyErr_Clear(); + continue; + } + const char* msgStr = PyBytes_AsString(msgValue); + logStrBuffer.emplace_back(msgValue); - const char* categoryStr = ""; - if (categoryAttr != Py_None) { - PyObject* categoryValue = PyUnicode_AsEncodedString(categoryAttr, "utf-8", nullptr); + const char* categoryStr = ""; + if (categoryAttr.get() != Py_None) { + PyObject* categoryValue = PyUnicode_AsEncodedString(categoryAttr.get(), "utf-8", "replace"); + if (categoryValue != nullptr) { categoryStr = PyBytes_AsString(categoryValue); logStrBuffer.emplace_back(categoryValue); } + } - if (PyObject_IsTrue(debugAttr)) { - const_cast(logger_).DebugLog(statusValue, categoryStr, msgStr); - } else { - const_cast(logger_).Log(statusValue, categoryStr, msgStr); - } + if (PyObject_IsTrue(debugAttr.get()) == 1) { + logger.DebugLog(statusValue, categoryStr, msgStr); + } else { + logger.Log(statusValue, categoryStr, msgStr); } - PyList_SetSlice(pMessages_, 0, size, nullptr); + PyErr_Clear(); } - Py_DECREF(debugField); - Py_DECREF(msgField); - Py_DECREF(categoryField); - Py_DECREF(statusField); + PyList_SetSlice(pMessages_, 0, size, nullptr); + + PyErr_Restore(errType, errValue, errTraceback); } -void PySlaveInstance::initialize(PyGILState_STATE gilState) +void PySlaveInstance::initialize() { - Py_XDECREF(pInstance_); - Py_XDECREF(pMessages_); + clearLogBuffer(); + Py_CLEAR(pInstance_); + Py_CLEAR(pMessages_); - PyObject* args = PyTuple_New(0); - PyObject* kwargs = Py_BuildValue("{ss,ss,sn,si}", + PyRef args(PyTuple_New(0)); + PyRef kwargs(Py_BuildValue("{ss,ss,sn,si}", "instance_name", instanceName_.c_str(), "resources", resources_.c_str(), - "logger", &logger_, - "visible", visible_); - pInstance_ = PyObject_Call(pClass_, args, kwargs); - Py_DECREF(args); - Py_DECREF(kwargs); - if (pInstance_ == nullptr) { - handle_py_exception("[initialize] PyObject_Call", gilState); - } + "logger", reinterpret_cast(&logger_), + "visible", static_cast(visible_))); + if (!args || !kwargs) handle_py_exception("[initialize] Py_BuildValue"); + + pInstance_ = PyObject_Call(pClass_, args.get(), kwargs.get()); + if (pInstance_ == nullptr) handle_py_exception("[initialize] PyObject_Call"); + pMessages_ = PyObject_CallMethod(pInstance_, "_get_log_queue", nullptr); + if (pMessages_ == nullptr) handle_py_exception("[initialize] _get_log_queue"); + clearLogBuffer(); +} + +void PySlaveInstance::callVoidMethod(const char* method) const +{ + PyRef result(PyObject_CallMethod(pInstance_, method, nullptr)); + if (!result) handle_py_exception(std::string("[") + method + "]"); + clearLogBuffer(); +} + +void PySlaveInstance::callSetter(const char* method, const cppfmu::FMIValueReference* vr, std::size_t nvr, PyObject* valuesList) +{ + PyRef values(valuesList); + if (!values) handle_py_exception(std::string("[") + method + "] building values"); + PyRef vrs(makeVrList(vr, nvr)); + if (!vrs) handle_py_exception(std::string("[") + method + "] building value references"); + + PyRef result(PyObject_CallMethod(pInstance_, method, "(OO)", vrs.get(), values.get())); + if (!result) handle_py_exception(std::string("[") + method + "]"); + clearLogBuffer(); +} + +PyObject* PySlaveInstance::callGetter(const char* method, const cppfmu::FMIValueReference* vr, std::size_t nvr, std::size_t nValues) const +{ + PyRef vrs(makeVrList(vr, nvr)); + if (!vrs) handle_py_exception(std::string("[") + method + "] building value references"); + + PyRef refs(PyObject_CallMethod(pInstance_, method, "(O)", vrs.get())); + if (!refs) handle_py_exception(std::string("[") + method + "]"); + checkListSize(refs.get(), nValues, method); + return refs.release(); +} + +PyObject* PySlaveInstance::callListMethod(const char* method, std::size_t expected) const +{ + PyRef refs(PyObject_CallMethod(pInstance_, method, nullptr)); + if (!refs) handle_py_exception(std::string("[") + method + "]"); + checkListSize(refs.get(), expected, method); + return refs.release(); +} + +void PySlaveInstance::checkListSize(PyObject* list, std::size_t expected, const char* method) const +{ + if (!PyList_Check(list)) { + clearLogBuffer(); + throw std::runtime_error(std::string("[") + method + "] must return a list"); + } + const auto actual = static_cast(PyList_Size(list)); + if (actual < expected) { + clearLogBuffer(); + std::ostringstream oss; + oss << "[" << method << "] returned " << actual << " values, but " << expected << " were requested"; + throw std::runtime_error(oss.str()); + } } void PySlaveInstance::SetupExperiment(cppfmu::FMIBoolean, cppfmu::FMIFloat64, cppfmu::FMIFloat64 startTime, cppfmu::FMIBoolean, cppfmu::FMIFloat64) { - py_safe_run([this, startTime](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "setup_experiment", "(d)", startTime); - if (f == nullptr) { - handle_py_exception("[setupExperiment] PyObject_CallMethod", gilState); - } - Py_DECREF(f); + py_safe_run([this, startTime]() { + PyRef result(PyObject_CallMethod(pInstance_, "setup_experiment", "(d)", startTime)); + if (!result) handle_py_exception("[setup_experiment]"); clearLogBuffer(); }); } void PySlaveInstance::EnterInitializationMode() { - py_safe_run([this](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "enter_initialization_mode", nullptr); - if (f == nullptr) { - handle_py_exception("[enterInitializationMode] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); - }); + py_safe_run([this]() { callVoidMethod("enter_initialization_mode"); }); } void PySlaveInstance::ExitInitializationMode() { - py_safe_run([this](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "exit_initialization_mode", nullptr); - if (f == nullptr) { - handle_py_exception("[exitInitializationMode] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); - }); + py_safe_run([this]() { callVoidMethod("exit_initialization_mode"); }); } cppfmu::FMIStatus PySlaveInstance::DoStep(cppfmu::FMIFloat64 currentTime, cppfmu::FMIFloat64 stepSize, - cppfmu::FMIBoolean noSetFmuStatePriorToCurrentPoint, + cppfmu::FMIBoolean, cppfmu::FMIBoolean* eventHandlingNeeded, cppfmu::FMIBoolean* terminateSimulation, cppfmu::FMIBoolean* earlyReturn, cppfmu::FMIFloat64& endOfStep) { - cppfmu::FMIStatus fmuStatus = cppfmu::FMIOK; - py_safe_run([this, &fmuStatus, currentTime, stepSize, terminateSimulation](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "do_step", "(dd)", currentTime, stepSize); - if (f == nullptr) { - handle_py_exception("[doStep] PyObject_CallMethod", gilState); - } + // The FMI standard requires these outputs to always be written. + if (eventHandlingNeeded) *eventHandlingNeeded = false; + if (terminateSimulation) *terminateSimulation = false; + if (earlyReturn) *earlyReturn = false; + + return py_safe_run([&]() { + cppfmu::FMIStatus fmuStatus = cppfmu::FMIOK; + PyRef result(PyObject_CallMethod(pInstance_, "do_step", "(dd)", currentTime, stepSize)); + if (!result) handle_py_exception("[do_step]"); + PyObject* f = result.get(); if (PyObject_HasAttrString(f, "status")) { - PyObject* pyStatus = PyObject_GetAttrString(f, "status"); - if (pyStatus) { - fmuStatus = static_cast(PyLong_AsLong(pyStatus)); - Py_DECREF(pyStatus); - } + PyRef pyStatus(PyObject_GetAttrString(f, "status")); + if (!pyStatus) handle_py_exception("[do_step] status"); + fmuStatus = static_cast(PyLong_AsLong(pyStatus.get())); + if (PyErr_Occurred()) handle_py_exception("[do_step] status conversion"); } else { - bool status = static_cast(PyObject_IsTrue(f)); - if (!status) { - fmuStatus = cppfmu::FMIDiscard; - } + const int ok = PyObject_IsTrue(f); + if (ok < 0) handle_py_exception("[do_step] return value"); + if (!ok) fmuStatus = cppfmu::FMIDiscard; } - if (PyObject_HasAttrString(f, "terminateSimulation")) { - PyObject* pyTerminateSimulation = PyObject_GetAttrString(f, "terminateSimulation"); - if (pyTerminateSimulation) { - *terminateSimulation = static_cast(PyObject_IsTrue(pyTerminateSimulation)); - Py_DECREF(pyTerminateSimulation); - } - } - Py_DECREF(f); + const int ehn = readBoolAttr(f, "eventHandlingNeeded", false); + const int term = readBoolAttr(f, "terminateSimulation", false); + const int early = readBoolAttr(f, "earlyReturn", false); + if (ehn < 0 || term < 0 || early < 0) handle_py_exception("[do_step] result"); + + if (eventHandlingNeeded) *eventHandlingNeeded = ehn == 1; + if (terminateSimulation) *terminateSimulation = term == 1; + if (earlyReturn) *earlyReturn = early == 1; + if (fmuStatus == cppfmu::FMIOK) endOfStep = currentTime + stepSize; + clearLogBuffer(); + return fmuStatus; }); - - return fmuStatus; } void PySlaveInstance::Reset() { - py_safe_run([this](PyGILState_STATE gilState) { - initialize(gilState); - }); + py_safe_run([this]() { initialize(); }); } void PySlaveInstance::Terminate() { - py_safe_run([this](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "terminate", nullptr); - if (f == nullptr) { - handle_py_exception("[terminate] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); - }); + py_safe_run([this]() { callVoidMethod("terminate"); }); } void PySlaveInstance::SetFloat64(const cppfmu::FMIValueReference* vr, std::size_t nvr, const cppfmu::FMIFloat64* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nValues); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, Py_BuildValue("d", values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_float64", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setFloat64] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_float64", vr, nvr, makeValueList(values, nValues, [](cppfmu::FMIFloat64 v) { return PyFloat_FromDouble(v); })); }); } void PySlaveInstance::SetInt32(const cppfmu::FMIValueReference* vr, std::size_t nvr, const cppfmu::FMIInt32* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nValues); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, Py_BuildValue("i", values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_int32", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setInt32] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_int32", vr, nvr, makeValueList(values, nValues, [](cppfmu::FMIInt32 v) { return PyLong_FromLong(v); })); }); } void PySlaveInstance::SetInt64(const cppfmu::FMIValueReference* vr, std::size_t nvr, const cppfmu::FMIInt64* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nValues); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, Py_BuildValue("L", values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_int64", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setInt64] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_int64", vr, nvr, makeValueList(values, nValues, [](cppfmu::FMIInt64 v) { return PyLong_FromLongLong(v); })); }); } void PySlaveInstance::SetUInt64(const cppfmu::FMIValueReference* vr, std::size_t nvr, const cppfmu::FMIUInt64* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nValues); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, Py_BuildValue("K", values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_uint64", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setUInt64] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_uint64", vr, nvr, makeValueList(values, nValues, [](cppfmu::FMIUInt64 v) { return PyLong_FromUnsignedLongLong(v); })); }); } void PySlaveInstance::SetBoolean(const cppfmu::FMIValueReference* vr, std::size_t nvr, const cppfmu::FMIBoolean* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nValues); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, PyBool_FromLong(values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_boolean", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setBoolean] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_boolean", vr, nvr, makeValueList(values, nValues, [](cppfmu::FMIBoolean v) { return PyBool_FromLong(v ? 1 : 0); })); }); } void PySlaveInstance::SetString(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIString const* values, std::size_t nValues) { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - PyObject* refs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - for (int i = 0; i < nValues; i++) { - PyList_SetItem(refs, i, Py_BuildValue("s", values[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_string", "(OO)", vrs, refs); - Py_DECREF(vrs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setString] PyObject_CallMethod", gilState); - } - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + callSetter("set_string", vr, nvr, makeValueList(values, nValues, makeString)); }); } void PySlaveInstance::GetFloat64(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIFloat64* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); + py_safe_run([&]() { + PyRef refs(callGetter("get_float64", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + values[i] = PyFloat_AsDouble(PyList_GetItem(refs.get(), i)); } - - auto refs = PyObject_CallMethod(pInstance_, "get_float64", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getFloat64] PyObject_CallMethod", gilState); - } - - for (int i = 0; i < nValues; i++) { - PyObject* value = PyList_GetItem(refs, i); - values[i] = PyFloat_AsDouble(value); - } - Py_DECREF(refs); + if (PyErr_Occurred()) handle_py_exception("[get_float64] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetInt32(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIInt32* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - auto refs = PyObject_CallMethod(pInstance_, "get_int32", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getInt32] PyObject_CallMethod", gilState); - } - - for (int i = 0; i < nValues; i++) { - PyObject* value = PyList_GetItem(refs, i); - values[i] = static_cast(PyLong_AsLong(value)); + py_safe_run([&]() { + PyRef refs(callGetter("get_int32", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + values[i] = static_cast(PyLong_AsLong(PyList_GetItem(refs.get(), i))); } - Py_DECREF(refs); + if (PyErr_Occurred()) handle_py_exception("[get_int32] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetInt64(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIInt64* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - auto refs = PyObject_CallMethod(pInstance_, "get_int64", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getInt64] PyObject_CallMethod", gilState); - } - - for (int i = 0; i < nValues; i++) { - PyObject* value = PyList_GetItem(refs, i); - values[i] = static_cast(PyLong_AsLongLong(value)); + py_safe_run([&]() { + PyRef refs(callGetter("get_int64", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + values[i] = static_cast(PyLong_AsLongLong(PyList_GetItem(refs.get(), i))); } - Py_DECREF(refs); + if (PyErr_Occurred()) handle_py_exception("[get_int64] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetUInt64(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIUInt64* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - auto refs = PyObject_CallMethod(pInstance_, "get_uint64", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getUInt64] PyObject_CallMethod", gilState); - } - - for (int i = 0; i < nValues; i++) { - PyObject* item = PyList_GetItem(refs, i); - PyObject* value = PyObject_GetAttrString(item, "value"); - if (value && PyLong_Check(value)) - { - values[i] = static_cast(PyLong_AsUnsignedLongLong(value)); + py_safe_run([&]() { + PyRef refs(callGetter("get_uint64", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + PyObject* item = PyList_GetItem(refs.get(), i); + if (PyLong_Check(item)) { + values[i] = static_cast(PyLong_AsUnsignedLongLong(item)); + } else { + // ctypes.c_uint64 (as returned by Fmi3SlaveBase.get_uint64) + PyRef value(PyObject_GetAttrString(item, "value")); + if (!value) handle_py_exception("[get_uint64] value attribute"); + values[i] = static_cast(PyLong_AsUnsignedLongLong(value.get())); } } - - Py_DECREF(refs); + if (PyErr_Occurred()) handle_py_exception("[get_uint64] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetBoolean(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIBoolean* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - auto refs = PyObject_CallMethod(pInstance_, "get_boolean", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getBoolean] PyObject_CallMethod", gilState); + py_safe_run([&]() { + PyRef refs(callGetter("get_boolean", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + const int v = PyObject_IsTrue(PyList_GetItem(refs.get(), i)); + if (v < 0) handle_py_exception("[get_boolean] conversion"); + values[i] = v == 1; } - - for (int i = 0; i < nValues; i++) { - PyObject* value = PyList_GetItem(refs, i); - values[i] = PyObject_IsTrue(value); - } - Py_DECREF(refs); clearLogBuffer(); }); } void PySlaveInstance::GetString(const cppfmu::FMIValueReference* vr, std::size_t nvr, cppfmu::FMIString* values, std::size_t nValues) const { - py_safe_run([this, &vr, nvr, &values, nValues](PyGILState_STATE gilState) { + py_safe_run([&]() { + // Strings returned to the importer stay valid until the next GetString call. clearStrBuffer(); - PyObject* vrs = PyList_New(nvr); - for (int i = 0; i < nvr; i++) { - PyList_SetItem(vrs, i, Py_BuildValue("i", vr[i])); - } - auto refs = PyObject_CallMethod(pInstance_, "get_string", "O", vrs); - Py_DECREF(vrs); - if (refs == nullptr) { - handle_py_exception("[getString] PyObject_CallMethod", gilState); - } - - for (int i = 0; i < nValues; i++) { - PyObject* value = PyUnicode_AsEncodedString(PyList_GetItem(refs, i), "utf-8", nullptr); + PyRef refs(callGetter("get_string", vr, nvr, nValues)); + for (std::size_t i = 0; i < nValues; i++) { + PyObject* value = PyUnicode_AsEncodedString(PyList_GetItem(refs.get(), i), "utf-8", nullptr); + if (value == nullptr) handle_py_exception("[get_string] conversion"); values[i] = PyBytes_AsString(value); strBuffer.emplace_back(value); } - Py_DECREF(refs); clearLogBuffer(); }); } void PySlaveInstance::GetContinuousStates(cppfmu::FMIFloat64* continuousStates, std::size_t nStates) const { - py_safe_run([this, &continuousStates, nStates](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_continuous_states", nullptr); - if (f == nullptr) { - handle_py_exception("[get_continuous_states] PyObject_CallMethod", gilState); - } - // Assuming f is a list of floats + py_safe_run([&]() { + PyRef refs(callListMethod("get_continuous_states", nStates)); for (std::size_t i = 0; i < nStates; i++) { - PyObject* item = PyList_GetItem(f, i); - continuousStates[i] = static_cast(PyFloat_AsDouble(item)); + continuousStates[i] = PyFloat_AsDouble(PyList_GetItem(refs.get(), i)); } - Py_DECREF(f); + if (PyErr_Occurred()) handle_py_exception("[get_continuous_states] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetContinuousStateDerivatives(cppfmu::FMIFloat64* continuousStateDerivatives, std::size_t nStates) const { - py_safe_run([this, &continuousStateDerivatives, nStates](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_continuous_state_derivatives", nullptr); - if (f == nullptr) { - PyErr_Print(); - handle_py_exception("[get_continuous_state_derivatives] PyObject_CallMethod", gilState); - } - // Assuming f is a list of floats + py_safe_run([&]() { + PyRef refs(callListMethod("get_continuous_state_derivatives", nStates)); for (std::size_t i = 0; i < nStates; i++) { - PyObject* item = PyList_GetItem(f, i); - continuousStateDerivatives[i] = static_cast(PyFloat_AsDouble(item)); + continuousStateDerivatives[i] = PyFloat_AsDouble(PyList_GetItem(refs.get(), i)); } - Py_DECREF(f); + if (PyErr_Occurred()) handle_py_exception("[get_continuous_state_derivatives] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetNominalsOfContinuousStates(cppfmu::FMIFloat64* nominalsOfContinuousStates, std::size_t nStates) const { - py_safe_run([this, &nominalsOfContinuousStates, nStates](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_nominals_of_continuous_states", "(i)", static_cast(nStates)); - // Check if f is a valid object - if (f == nullptr) { - handle_py_exception("[getNominalContinuousStates] PyObject_CallMethod", gilState); - } - // Assuming f is a list of floats + py_safe_run([&]() { + PyRef refs(PyObject_CallMethod(pInstance_, "get_nominals_of_continuous_states", "(n)", static_cast(nStates))); + if (!refs) handle_py_exception("[get_nominals_of_continuous_states]"); + checkListSize(refs.get(), nStates, "get_nominals_of_continuous_states"); for (std::size_t i = 0; i < nStates; i++) { - PyObject* item = PyList_GetItem(f, i); - nominalsOfContinuousStates[i] = static_cast(PyFloat_AsDouble(item)); + nominalsOfContinuousStates[i] = PyFloat_AsDouble(PyList_GetItem(refs.get(), i)); } - Py_DECREF(f); + if (PyErr_Occurred()) handle_py_exception("[get_nominals_of_continuous_states] conversion"); clearLogBuffer(); }); } void PySlaveInstance::SetContinuousStates(const cppfmu::FMIFloat64* continuousStates, std::size_t nStates) { - py_safe_run([this, &continuousStates, nStates](PyGILState_STATE gilState) { - PyObject* refs = PyList_New(nStates); - for (int i = 0; i < nStates; i++) { - PyList_SetItem(refs, i, Py_BuildValue("d", continuousStates[i])); - } - - auto f = PyObject_CallMethod(pInstance_, "set_continuous_states", "(O)", refs); - Py_DECREF(refs); - if (f == nullptr) { - handle_py_exception("[setContinuousStates] PyObject_CallMethod", gilState); - } - Py_DECREF(f); + py_safe_run([&]() { + PyRef refs(makeValueList(continuousStates, nStates, [](cppfmu::FMIFloat64 v) { return PyFloat_FromDouble(v); })); + if (!refs) handle_py_exception("[set_continuous_states] building values"); + PyRef result(PyObject_CallMethod(pInstance_, "set_continuous_states", "(O)", refs.get())); + if (!result) handle_py_exception("[set_continuous_states]"); clearLogBuffer(); }); } void PySlaveInstance::GetNumberOfContinuousStates(std::size_t& nStates) const { - py_safe_run([this, &nStates](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_number_of_continuous_states", nullptr); - if (f == nullptr) { - handle_py_exception("[getNumberOfContinuousStates] PyObject_CallMethod", gilState); - } - nStates = static_cast(PyLong_AsLong(f)); - Py_DECREF(f); + py_safe_run([&]() { + PyRef result(PyObject_CallMethod(pInstance_, "get_number_of_continuous_states", nullptr)); + if (!result) handle_py_exception("[get_number_of_continuous_states]"); + nStates = PyLong_AsSize_t(result.get()); + if (PyErr_Occurred()) handle_py_exception("[get_number_of_continuous_states] conversion"); clearLogBuffer(); }); } void PySlaveInstance::GetNumberOfEventIndicators(std::size_t& nIndicators) const { - py_safe_run([this, &nIndicators](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_number_of_event_indicators", nullptr); - if (f == nullptr) { - handle_py_exception("[getNumberOfEventIndicators] PyObject_CallMethod", gilState); - } - nIndicators = static_cast(PyLong_AsLong(f)); - Py_DECREF(f); - clearLogBuffer(); + py_safe_run([&]() { + PyRef result(PyObject_CallMethod(pInstance_, "get_number_of_event_indicators", nullptr)); + if (!result) handle_py_exception("[get_number_of_event_indicators]"); + nIndicators = PyLong_AsSize_t(result.get()); + if (PyErr_Occurred()) handle_py_exception("[get_number_of_event_indicators] conversion"); + clearLogBuffer(); }); } void PySlaveInstance::GetEventIndicators(cppfmu::FMIFloat64* eventIndicators, std::size_t nIndicators) const { - py_safe_run([this, &eventIndicators, nIndicators](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "get_event_indicators", nullptr); - if (f == nullptr) { - handle_py_exception("[getEventIndicators] PyObject_CallMethod", gilState); - } - // Assuming f is a list of floats + py_safe_run([&]() { + PyRef refs(callListMethod("get_event_indicators", nIndicators)); for (std::size_t i = 0; i < nIndicators; i++) { - PyObject* item = PyList_GetItem(f, i); - eventIndicators[i] = static_cast(PyFloat_AsDouble(item)); + eventIndicators[i] = PyFloat_AsDouble(PyList_GetItem(refs.get(), i)); } - Py_DECREF(f); + if (PyErr_Occurred()) handle_py_exception("[get_event_indicators] conversion"); clearLogBuffer(); }); - } - void PySlaveInstance::SetTime(cppfmu::FMIFloat64 time) { - py_safe_run([this, time](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "set_time", "(d)", time); - if (f == nullptr) { - handle_py_exception("[setTime] PyObject_CallMethod", gilState); - } - Py_DECREF(f); + py_safe_run([this, time]() { + PyRef result(PyObject_CallMethod(pInstance_, "set_time", "(d)", time)); + if (!result) handle_py_exception("[set_time]"); clearLogBuffer(); }); } @@ -677,174 +675,132 @@ void PySlaveInstance::UpdateDiscreteStates( cppfmu::FMIBoolean* nextEventTimeDefined, cppfmu::FMIFloat64* nextEventTime) { - py_safe_run([this, discreteStatesNeedUpdate, terminateSimulation, nominalContinuousStatesChanged, valuesOfContinuousStatesChanged, nextEventTimeDefined, nextEventTime](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "update_discrete_states", nullptr); - if (f == nullptr) { - handle_py_exception("[updateDiscreteStates] PyObject_CallMethod", gilState); - } - - if (PyObject_HasAttrString(f, "discreteStatesNeedUpdate")) { - PyObject * pyDiscreteStatesNeedUpdate = PyObject_GetAttrString(f, "discreteStatesNeedUpdate"); - *discreteStatesNeedUpdate = static_cast(PyObject_IsTrue(pyDiscreteStatesNeedUpdate)); - Py_DECREF(pyDiscreteStatesNeedUpdate); - } - if (PyObject_HasAttrString(f, "terminateSimulation")) { - PyObject* pyTerminateSimulation = PyObject_GetAttrString(f, "terminateSimulation"); - *terminateSimulation = static_cast(PyObject_IsTrue(pyTerminateSimulation)); - Py_DECREF(pyTerminateSimulation); - } - if (PyObject_HasAttrString(f, "nominalContinuousStatesChanged")) { - PyObject* pyNominalContinuousStatesChanged = PyObject_GetAttrString(f, "nominalContinuousStatesChanged"); - *nominalContinuousStatesChanged = static_cast(PyObject_IsTrue(pyNominalContinuousStatesChanged)); - Py_DECREF(pyNominalContinuousStatesChanged); - } - if (PyObject_HasAttrString(f, "valuesOfContinuousStatesChanged")) { - PyObject* pyValuesOfContinuousStatesChanged = PyObject_GetAttrString(f, "valuesOfContinuousStatesChanged"); - *valuesOfContinuousStatesChanged = static_cast(PyObject_IsTrue(pyValuesOfContinuousStatesChanged)); - Py_DECREF(pyValuesOfContinuousStatesChanged); - } - if (PyObject_HasAttrString(f, "nextEventTimeDefined")) { - PyObject* pyNextEventTimeDefined = PyObject_GetAttrString(f, "nextEventTimeDefined"); - *nextEventTimeDefined = static_cast(PyObject_IsTrue(pyNextEventTimeDefined)); - Py_DECREF(pyNextEventTimeDefined); + py_safe_run([&]() { + PyRef result(PyObject_CallMethod(pInstance_, "update_discrete_states", nullptr)); + if (!result) handle_py_exception("[update_discrete_states]"); + PyObject* f = result.get(); + + struct Field + { + const char* name; + cppfmu::FMIBoolean* out; + }; + const Field fields[] = { + {"discreteStatesNeedUpdate", discreteStatesNeedUpdate}, + {"terminateSimulation", terminateSimulation}, + {"nominalContinuousStatesChanged", nominalContinuousStatesChanged}, + {"valuesOfContinuousStatesChanged", valuesOfContinuousStatesChanged}, + {"nextEventTimeDefined", nextEventTimeDefined}, + }; + for (const auto& field : fields) { + const int v = readBoolAttr(f, field.name, false); + if (v < 0) handle_py_exception(std::string("[update_discrete_states] ") + field.name); + if (field.out) *field.out = v == 1; + } + + if (nextEventTimeDefined && *nextEventTimeDefined && nextEventTime) { + PyRef pyNextEventTime(PyObject_GetAttrString(f, "nextEventTime")); + if (!pyNextEventTime) handle_py_exception("[update_discrete_states] nextEventTime"); + *nextEventTime = PyFloat_AsDouble(pyNextEventTime.get()); + if (PyErr_Occurred()) handle_py_exception("[update_discrete_states] nextEventTime conversion"); } - if (nextEventTimeDefined && *nextEventTimeDefined) { - PyObject* pyNextEventTime = PyObject_GetAttrString(f, "nextEventTime"); - *nextEventTime = PyFloat_AsDouble(pyNextEventTime); - Py_DECREF(pyNextEventTime); - } - - Py_DECREF(f); clearLogBuffer(); }); } void PySlaveInstance::GetFMUstate(fmi3FMUState& state) { - py_safe_run([this, &state](PyGILState_STATE gilState) { - auto f = PyObject_CallMethod(pInstance_, "_get_fmu_state", nullptr); - if (f == nullptr) { - handle_py_exception("[_get_fmu_state] PyObject_CallMethod", gilState); - } - state = reinterpret_cast(f); + py_safe_run([&]() { + PyRef newState(PyObject_CallMethod(pInstance_, "_get_fmu_state", nullptr)); + if (!newState) handle_py_exception("[_get_fmu_state]"); + // FMI 3.0: a non-NULL state was returned by an earlier call and must be overwritten. + PyObject* previous = reinterpret_cast(state); + state = reinterpret_cast(newState.release()); + Py_XDECREF(previous); clearLogBuffer(); }); } void PySlaveInstance::SetFMUstate(const fmi3FMUState& state) { - py_safe_run([this, &state](PyGILState_STATE gilState) { + py_safe_run([&]() { auto pyState = reinterpret_cast(state); - auto f = PyObject_CallMethod(pInstance_, "_set_fmu_state", "(O)", pyState); - if (f == nullptr) { - handle_py_exception("[_set_fmu_state] PyObject_CallMethod", gilState); - } + PyRef result(PyObject_CallMethod(pInstance_, "_set_fmu_state", "(O)", pyState)); + if (!result) handle_py_exception("[_set_fmu_state]"); clearLogBuffer(); }); } void PySlaveInstance::FreeFMUstate(fmi3FMUState& state) { - py_safe_run([this, &state](PyGILState_STATE gilState) { - auto f = reinterpret_cast(state); - Py_XDECREF(f); + py_safe_run([&]() { + Py_XDECREF(reinterpret_cast(state)); + state = nullptr; }); } size_t PySlaveInstance::SerializedFMUstateSize(const fmi3FMUState& state) { - size_t size; - py_safe_run([this, &state, &size](PyGILState_STATE gilState) { + return py_safe_run([&]() { auto pyState = reinterpret_cast(state); - PyObject* pyStateBytes = PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState); - if (pyStateBytes == nullptr) { - handle_py_exception("[SerializedFMUstateSize] PyObject_CallMethod", gilState); - } - size = PyBytes_Size(pyStateBytes); - Py_DECREF(pyStateBytes); + PyRef bytes(PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState)); + if (!bytes) handle_py_exception("[_fmu_state_to_bytes]"); + const Py_ssize_t size = PyBytes_Size(bytes.get()); + if (size < 0) handle_py_exception("[_fmu_state_to_bytes] must return bytes"); clearLogBuffer(); + return static_cast(size); }); - return size; } void PySlaveInstance::SerializeFMUstate(const fmi3FMUState& state, fmi3Byte* bytes, size_t size) { - py_safe_run([this, &state, &bytes, size](PyGILState_STATE gilState) { + py_safe_run([&]() { auto pyState = reinterpret_cast(state); - PyObject* pyStateBytes = PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState); - if (pyStateBytes == nullptr) { - handle_py_exception("[SerializeFMUstate] PyObject_CallMethod", gilState); - } - char* c = PyBytes_AsString(pyStateBytes); - if (c == nullptr) { - handle_py_exception("[SerializeFMUstate] PyBytes_AsString", gilState); + PyRef pyBytes(PyObject_CallMethod(pClass_, "_fmu_state_to_bytes", "(O)", pyState)); + if (!pyBytes) handle_py_exception("[_fmu_state_to_bytes]"); + char* data = nullptr; + Py_ssize_t length = 0; + if (PyBytes_AsStringAndSize(pyBytes.get(), &data, &length) != 0) { + handle_py_exception("[_fmu_state_to_bytes] must return bytes"); } - for (int i = 0; i < size; i++) { - bytes[i] = c[i]; + if (static_cast(length) > size) { + throw std::runtime_error("[SerializeFMUstate] buffer is smaller than fmi3SerializedFMUStateSize"); } - Py_DECREF(pyStateBytes); + std::copy(data, data + length, bytes); clearLogBuffer(); }); } void PySlaveInstance::DeSerializeFMUstate(const fmi3Byte bytes[], size_t size, fmi3FMUState& state) { - py_safe_run([this, &bytes, size, &state](PyGILState_STATE gilState) { - char const * castedBytes = reinterpret_cast(bytes); - PyObject* pyStateBytes = PyBytes_FromStringAndSize(castedBytes, size); - if (pyStateBytes == nullptr) { - handle_py_exception("[DeSerializeFMUstate] PyBytes_FromStringAndSize", gilState); - } - PyObject* pyState = PyObject_CallMethod(pClass_, "_fmu_state_from_bytes", "(O)", pyStateBytes); - if (pyState == nullptr) { - handle_py_exception("[DeSerializeFMUstate] PyObject_CallMethod", gilState); - } - state = reinterpret_cast(pyState); - Py_DECREF(pyStateBytes); + py_safe_run([&]() { + PyRef pyBytes(PyBytes_FromStringAndSize(reinterpret_cast(bytes), static_cast(size))); + if (!pyBytes) handle_py_exception("[DeSerializeFMUstate] PyBytes_FromStringAndSize"); + PyRef pyState(PyObject_CallMethod(pClass_, "_fmu_state_from_bytes", "(O)", pyBytes.get())); + if (!pyState) handle_py_exception("[_fmu_state_from_bytes]"); + PyObject* previous = reinterpret_cast(state); + state = reinterpret_cast(pyState.release()); + Py_XDECREF(previous); clearLogBuffer(); }); } -void PySlaveInstance::handle_py_exception(const std::string& what, PyGILState_STATE gilState) const +void PySlaveInstance::handle_py_exception(const std::string& what) const { - auto err = PyErr_Occurred(); - if (err != nullptr) { - cleanPyObject(); - - PyObject *pExcType, *pExcValue, *pExcTraceback; - PyErr_Fetch(&pExcType, &pExcValue, &pExcTraceback); - - 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"; - } + // Must be called with the GIL held. The GIL is released by the enclosing GilGuard + // while the exception unwinds; Python objects stay valid so the instance can still + // be freed (or reset) safely afterwards. + std::string details = fetchPythonError(); + clearLogBuffer(); // forward messages logged before the failure - PyErr_Clear(); - - Py_XDECREF(pExcType); - Py_XDECREF(pExcValue); - Py_XDECREF(pExcTraceback); - - PyGILState_Release(gilState); - - auto msg = oss.str(); - throw cppfmu::FatalError(msg.c_str()); - } + std::ostringstream oss; + oss << "Fatal py exception encountered: " << what << "\n" << details; + throw cppfmu::FatalError(oss.str().c_str()); } PySlaveInstance::~PySlaveInstance() { - py_safe_run([this](PyGILState_STATE gilState) { - cleanPyObject(); - }); + py_safe_run([this]() { cleanPyObject(); }); } } // namespace pythonfmu @@ -866,6 +822,34 @@ namespace { static auto* const state = new std::shared_ptr(); return *state; } + + // Turns an FMI 3 resource path (or a legacy file:// URI) into a filesystem path. + std::string resourcePathFromLocation(const std::string& location) + { + std::string path = location; + const std::string scheme = "file://"; + if (path.compare(0, scheme.size(), scheme) == 0) { + path.erase(0, scheme.size()); + // file:///C:/... on Windows -> C:/... + if (path.size() > 2 && path[0] == '/' && path[2] == ':') path.erase(0, 1); + + // Percent-decode (URIs only; plain FMI 3 paths may legitimately contain '%') + std::string decoded; + decoded.reserve(path.size()); + for (std::size_t i = 0; i < path.size(); i++) { + if (path[i] == '%' && i + 2 < path.size() && std::isxdigit(static_cast(path[i + 1])) && + std::isxdigit(static_cast(path[i + 2]))) { + decoded += static_cast(std::stoi(path.substr(i + 1, 2), nullptr, 16)); + i += 2; + } else { + decoded += path[i]; + } + } + path = decoded; + } + while (path.size() > 1 && (path.back() == '/' || path.back() == '\\')) path.pop_back(); + return path; + } } std::unique_ptr CppfmuInstantiateSlave( @@ -878,29 +862,19 @@ std::unique_ptr CppfmuInstantiateSlave( cppfmu::FMIBoolean, const cppfmu::Logger& logger) { - - // Convert URI %20 to space - auto resources = std::regex_replace(std::string(fmuResourceLocation), std::regex("%20"), " "); - auto find = resources.find("file://"); - - if (find != std::string::npos) { -#ifdef _MSC_VER - resources.replace(find, 8, ""); -#else - resources.replace(find, 7, ""); -#endif + if (fmuResourceLocation == nullptr) { + throw cppfmu::FatalError("No resource path given: PythonFMU3 needs the FMU resources folder"); } + auto resources = resourcePathFromLocation(fmuResourceLocation); - { - auto const ensurePyStateAlive = [&]() { - auto const lock = std::lock_guard{pyStateMutex()}; - if (nullptr == pyState()) pyState() = std::make_shared(); - return pyState(); - }; - - return std::make_unique( - instanceName, resources, logger, visible, ensurePyStateAlive()); - } + auto const ensurePyStateAlive = [&]() { + auto const lock = std::lock_guard{pyStateMutex()}; + if (nullptr == pyState()) pyState() = std::make_shared(); + return pyState(); + }; + + return std::make_unique( + instanceName, resources, logger, visible, ensurePyStateAlive()); } extern "C" { diff --git a/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.hpp b/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.hpp index 609ca61..03a7905 100644 --- a/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.hpp +++ b/pythonfmu3/pythonfmu-export/src/pythonfmu/PySlaveInstance.hpp @@ -18,7 +18,8 @@ class PySlaveInstance : public cppfmu::SlaveInstance public: PySlaveInstance(std::string instanceName, std::string resources, const cppfmu::Logger& logger, bool visible, std::shared_ptr pyState); - void initialize(PyGILState_STATE gilState); + // Must be called with the GIL held. + void initialize(); void SetupExperiment(cppfmu::FMIBoolean toleranceDefined, cppfmu::FMIFloat64 tolerance, cppfmu::FMIFloat64 tStart, cppfmu::FMIBoolean stopTimeDefined, cppfmu::FMIFloat64 tStop) override; void EnterInitializationMode() override; @@ -79,7 +80,7 @@ class PySlaveInstance : public cppfmu::SlaveInstance private: std::shared_ptr pyState_; - PyObject* pClass_; + PyObject* pClass_{}; PyObject* pInstance_{}; PyObject* pMessages_{}; @@ -91,7 +92,18 @@ class PySlaveInstance : public cppfmu::SlaveInstance mutable std::vector strBuffer; mutable std::vector logStrBuffer; - void handle_py_exception(const std::string& what, PyGILState_STATE gilState) const; + // Formats the pending Python exception (with traceback) and throws cppfmu::FatalError. + // Must be called with the GIL held; never returns. + [[noreturn]] void handle_py_exception(const std::string& what) const; + + // Helpers shared by the getters/setters. All require the GIL. + void callVoidMethod(const char* method) const; + // Takes ownership of valuesList (may be nullptr if building it failed). + void callSetter(const char* method, const cppfmu::FMIValueReference* vr, std::size_t nvr, PyObject* valuesList); + // Return new references to lists holding at least the requested number of items. + PyObject* callGetter(const char* method, const cppfmu::FMIValueReference* vr, std::size_t nvr, std::size_t nValues) const; + PyObject* callListMethod(const char* method, std::size_t expected) const; + void checkListSize(PyObject* list, std::size_t expected, const char* method) const; inline void clearStrBuffer() const { @@ -113,14 +125,15 @@ class PySlaveInstance : public cppfmu::SlaveInstance } } - inline void cleanPyObject() const + // Releases every Python reference held by this instance. Safe to call more than once. + inline void cleanPyObject() { clearLogBuffer(); clearLogStrBuffer(); clearStrBuffer(); - Py_XDECREF(pClass_); - Py_XDECREF(pInstance_); - Py_XDECREF(pMessages_); + Py_CLEAR(pClass_); + Py_CLEAR(pInstance_); + Py_CLEAR(pMessages_); } }; diff --git a/pythonfmu3/tests/fmi3_ctypes_driver.py b/pythonfmu3/tests/fmi3_ctypes_driver.py new file mode 100644 index 0000000..7555648 --- /dev/null +++ b/pythonfmu3/tests/fmi3_ctypes_driver.py @@ -0,0 +1,137 @@ +"""Minimal ctypes FMI 3 co-simulation driver used by test_wrapper_ctypes.py. + +usage: python fmi3_ctypes_driver.py +""" +import ctypes as C +import sys +from pathlib import Path + +scenario, fmu_dir = sys.argv[1], Path(sys.argv[2]) +lib_path = next((fmu_dir / "binaries").rglob("*.so")) +lib = C.CDLL(str(lib_path)) + +LOGCB = C.CFUNCTYPE(None, C.c_void_p, C.c_int, C.c_char_p, C.c_char_p) +logs = [] +@LOGCB +def logger(env, status, category, message): + logs.append((status, (category or b"").decode(), message.decode())) + +inst_fn = lib.fmi3InstantiateCoSimulation +inst_fn.restype = C.c_void_p +inst_fn.argtypes = [C.c_char_p, C.c_char_p, C.c_char_p, C.c_bool, C.c_bool, C.c_bool, C.c_bool, + C.c_void_p, C.c_size_t, C.c_void_p, LOGCB, C.c_void_p] +lib.fmi3FreeInstance.argtypes = [C.c_void_p] +for name in ["fmi3EnterInitializationMode"]: + getattr(lib, name).argtypes = [C.c_void_p, C.c_bool, C.c_double, C.c_double, C.c_bool, C.c_double] +for name in ["fmi3ExitInitializationMode", "fmi3Terminate", "fmi3Reset"]: + getattr(lib, name).argtypes = [C.c_void_p] +lib.fmi3DoStep.argtypes = [C.c_void_p, C.c_double, C.c_double, C.c_bool, + C.POINTER(C.c_bool), C.POINTER(C.c_bool), C.POINTER(C.c_bool), C.POINTER(C.c_double)] +for t, ct in [("Float64", C.c_double), ("Int32", C.c_int32), ("Boolean", C.c_bool), ("String", C.c_char_p)]: + for op in ("Get", "Set"): + getattr(lib, f"fmi3{op}{t}").argtypes = [C.c_void_p, C.POINTER(C.c_uint32), C.c_size_t, C.POINTER(ct), C.c_size_t] +lib.fmi3GetFMUState.argtypes = [C.c_void_p, C.POINTER(C.c_void_p)] +lib.fmi3SetFMUState.argtypes = [C.c_void_p, C.c_void_p] +lib.fmi3FreeFMUState.argtypes = [C.c_void_p, C.POINTER(C.c_void_p)] + +def vrs(*v): + return (C.c_uint32 * len(v))(*v) + +def instantiate(logcb=logger, resources=None): + res = str(fmu_dir / "resources") if resources is None else resources + return inst_fn(b"inst", b"", res.encode(), False, True, False, False, None, 0, None, logcb, None) + +def step(c, t, dt): + ehn, term, early, last = C.c_bool(True), C.c_bool(True), C.c_bool(True), C.c_double(-1) + st = lib.fmi3DoStep(c, t, dt, True, C.byref(ehn), C.byref(term), C.byref(early), C.byref(last)) + return st, ehn.value, term.value, early.value, last.value + +# value references for tests/slaves/pythonslave.py +VR_INTPARAM, VR_REALIN, VR_BOOLPARAM, VR_STRPARAM, VR_REALOUT = 1, 2, 3, 4, 6 + +if scenario == "basic": + c = instantiate() + assert c + assert lib.fmi3EnterInitializationMode(c, False, 0, 0, False, 0) == 0 + assert lib.fmi3ExitInitializationMode(c) == 0 + st, ehn, term, early, last = step(c, 0.0, 0.5) + assert st == 0 and (ehn, term, early) == (False, False, False), (st, ehn, term, early) + assert abs(last - 0.5) < 1e-12, last + out = (C.c_double * 1)() + assert lib.fmi3GetFloat64(c, vrs(VR_REALOUT), 1, out, 1) == 0 + assert out[0] == 0.5, out[0] + iv = (C.c_int32 * 1)(7) + assert lib.fmi3SetInt32(c, vrs(VR_INTPARAM), 1, iv, 1) == 0 + iv2 = (C.c_int32 * 1)() + assert lib.fmi3GetInt32(c, vrs(VR_INTPARAM), 1, iv2, 1) == 0 and iv2[0] == 7 + sv = (C.c_char_p * 1)(b"cat") + assert lib.fmi3SetString(c, vrs(VR_STRPARAM), 1, sv, 1) == 0 + sv2 = (C.c_char_p * 1)() + assert lib.fmi3GetString(c, vrs(VR_STRPARAM), 1, sv2, 1) == 0 and sv2[0] == b"cat" + bv = (C.c_bool * 1)(True) + assert lib.fmi3SetBoolean(c, vrs(VR_BOOLPARAM), 1, bv, 1) == 0 + bv2 = (C.c_bool * 1)() + assert lib.fmi3GetBoolean(c, vrs(VR_BOOLPARAM), 1, bv2, 1) == 0 and bv2[0] + # FMU state round-trip, including overwriting an existing state + state = C.c_void_p() + assert lib.fmi3GetFMUState(c, C.byref(state)) == 0 and state.value + step(c, 0.5, 0.5) + assert lib.fmi3GetFMUState(c, C.byref(state)) == 0 # overwrite (old state must be released) + assert lib.fmi3SetFMUState(c, state) == 0 + assert lib.fmi3FreeFMUState(c, C.byref(state)) == 0 + assert lib.fmi3Reset(c) == 0 + assert lib.fmi3Terminate(c) == 0 + lib.fmi3FreeInstance(c) + print("OK basic") + +elif scenario == "bad_vr_type": + # Asking Float64 for an Int32 variable raises TypeError in Python -> error status, no crash + c = instantiate() + out = (C.c_double * 1)() + st = lib.fmi3GetFloat64(c, vrs(VR_INTPARAM), 1, out, 1) + assert st in (3, 4), st + assert any("TypeError" in m for _, _, m in logs), logs + lib.fmi3FreeInstance(c) # previously: double Py_DECREF after an exception + print("OK bad_vr_type", "traceback logged:", any("Traceback" in m for _, _, m in logs)) + +elif scenario == "too_few_values": + # nValues larger than what Python returns -> was an out-of-bounds read + c = instantiate() + out = (C.c_double * 5)() + st = lib.fmi3GetFloat64(c, vrs(VR_REALOUT), 1, out, 5) + assert st != 0, st + lib.fmi3FreeInstance(c) + print("OK too_few_values", logs[-1][2][:80]) + +elif scenario == "exception_in_step": + c = instantiate() + lib.fmi3EnterInitializationMode(c, False, 0, 0, False, 0) + lib.fmi3ExitInitializationMode(c) + st = step(c, 0.0, 0.1)[0] + assert st == 4, st # fatal + assert any("RuntimeError" in m for _, _, m in logs), logs + lib.fmi3FreeInstance(c) + print("OK exception_in_step") + +elif scenario == "null_logger": + c = inst_fn(b"inst", b"", str(fmu_dir / "resources").encode(), False, True, False, False, + None, 0, None, C.cast(None, LOGCB), None) + assert c + out = (C.c_double * 5)() + lib.fmi3GetFloat64(c, vrs(VR_REALOUT), 1, out, 5) # error path logs -> must not call NULL + lib.fmi3FreeInstance(c) + print("OK null_logger") + +elif scenario == "bad_resources": + c = instantiate(resources="/does/not/exist") + assert not c + print("OK bad_resources", logs[-1][2][:60] if logs else "") + +elif scenario == "uri_resources": + c = instantiate(resources="file://" + str(fmu_dir / "resources").replace(" ", "%20") + "/") + assert c + lib.fmi3FreeInstance(c) + print("OK uri_resources") + +else: + raise SystemExit(f"unknown scenario {scenario}") diff --git a/pythonfmu3/tests/test_builder.py b/pythonfmu3/tests/test_builder.py index 09de7df..9fd73eb 100644 --- a/pythonfmu3/tests/test_builder.py +++ b/pythonfmu3/tests/test_builder.py @@ -68,7 +68,7 @@ def test_zip_content(tmp_path): assert len(names) >= nfiles # Library + python script + XML + module name + sources with files.open(module_file) as myfile: - assert myfile.read() == b"pythonslave" + assert myfile.read().decode().splitlines() == ["pythonslave", "PythonSlave"] @pytest.mark.parametrize("pfiles", PROJECT_TEST_CASES) @@ -193,3 +193,49 @@ def build(): names = files.namelist() assert "terminalsAndIcons/terminalsAndIcons.xml" in names + + +def test_terminals_file_is_embedded(tmp_path): + script_file = Path(__file__).parent / "slaves/pythonslave.py" + terminals = tmp_path / "terminalsAndIcons.xml" + terminals.write_text("") + doc = tmp_path / "doc" + doc.mkdir() + (doc / "index.html").write_text("") + + fmu = FmuBuilder.build_FMU( + script_file, dest=tmp_path, documentation_folder=doc, terminals=terminals + ) + with zipfile.ZipFile(fmu) as files: + assert files.read("terminalsAndIcons/terminalsAndIcons.xml") == terminals.read_bytes() + + +def test_default_cosim_options_in_model_description(tmp_path): + from xml.etree.ElementTree import parse + from pythonfmu3.fmi3slave import FMI3_MODEL_OPTIONS_COSIM + + script_file = Path(__file__).parent / "slaves/pythonslave.py" + fmu = FmuBuilder.build_FMU(script_file, dest=tmp_path) + with zipfile.ZipFile(fmu) as files: + cs = parse(files.open("modelDescription.xml")).getroot().find("CoSimulation") + for option in FMI3_MODEL_OPTIONS_COSIM: + assert cs.get(option.name) == str(option.value).lower() + + +def test_class_with_multiline_bases(tmp_path): + script = tmp_path / "multiline.py" + script.write_text( + "from pythonfmu3 import Fmi3Slave, Float64, Fmi3Causality\n" + "class Multi(\n" + " Fmi3Slave,\n" + "):\n" + " def __init__(self, **kwargs):\n" + " super().__init__(**kwargs)\n" + " self.x = 1.0\n" + " self.register_variable(Float64('x', causality=Fmi3Causality.output))\n" + " def do_step(self, t, dt):\n" + " return True\n" + ) + fmu = FmuBuilder.build_FMU(script, dest=tmp_path) + with zipfile.ZipFile(fmu) as files: + assert files.read("resources/slavemodule.txt").decode().splitlines() == ["multiline", "Multi"] diff --git a/pythonfmu3/tests/test_variables.py b/pythonfmu3/tests/test_variables.py index 142b8a2..a7e138a 100644 --- a/pythonfmu3/tests/test_variables.py +++ b/pythonfmu3/tests/test_variables.py @@ -324,7 +324,7 @@ def test_array_to_xml(name, start, dims): ]) def test_array_output_to_xml(name, start, dims): import numpy as np - start = np.reshape(start, newshape=dims) + start = np.reshape(start, dims) r = Float64(name, start, dimensions=[Dimension(start=val) for val in dims], causality=Fmi3Causality.output) xml = r.to_xml() if start is not None: diff --git a/pythonfmu3/tests/test_wrapper_ctypes.py b/pythonfmu3/tests/test_wrapper_ctypes.py new file mode 100644 index 0000000..d0468f1 --- /dev/null +++ b/pythonfmu3/tests/test_wrapper_ctypes.py @@ -0,0 +1,61 @@ +"""Exercise the native wrapper directly through ctypes (no fmpy needed). + +Each scenario runs in its own interpreter so a crash in the wrapper fails the test +instead of taking down the test session. +""" +import subprocess +import sys +import zipfile +from pathlib import Path + +import pytest + +from pythonfmu3.builder import FmuBuilder + +pytestmark = pytest.mark.skipif( + not FmuBuilder.has_binary() or not sys.platform.startswith("linux"), + reason="needs the compiled wrapper (ctypes driver is Linux-only)", +) + +HERE = Path(__file__).parent +DRIVER = HERE / "fmi3_ctypes_driver.py" + + +def _extract(script: Path, dest: Path) -> Path: + fmu = FmuBuilder.build_FMU(script, dest=dest) + target = dest / fmu.stem + with zipfile.ZipFile(fmu) as z: + z.extractall(target) + return target + + +@pytest.fixture(scope="module") +def python_slave(tmp_path_factory): + # A space in the path exercises the resource-path handling + return _extract(HERE / "slaves/pythonslave.py", tmp_path_factory.mktemp("with space")) + + +@pytest.fixture(scope="module") +def exception_slave(tmp_path_factory): + return _extract(HERE / "slaves/PythonSlaveWithException.py", tmp_path_factory.mktemp("exc")) + + +def _run(scenario: str, fmu_dir: Path): + proc = subprocess.run( + [sys.executable, str(DRIVER), scenario, str(fmu_dir)], + capture_output=True, text=True, timeout=120, + ) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert f"OK {scenario}" in proc.stdout + + +@pytest.mark.parametrize( + "scenario", + ["basic", "bad_vr_type", "too_few_values", "null_logger", "bad_resources", "uri_resources"], +) +def test_wrapper(scenario, python_slave): + _run(scenario, python_slave) + + +def test_wrapper_exception_in_step(exception_slave): + _run("exception_in_step", exception_slave)