Fix wrapper memory-safety bugs and builder option/terminals bugs

pythonfmu3/builder.py

@@ -1,6 +1,6 @@
 """Python FMU builder"""
 import argparse
-import importlib
+import importlib.util
 import itertools
 import logging
 import re
@@ -21,21 +21,57 @@
 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:
         filepath (pathlib.Path) : script file path
         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"))

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):

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(

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<Component>(
             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<Component>(
             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)
         {