[#1433] Keep Msg_C reader subscriptions alive (module-scoped keep-alive)

docs/source/Learn/bskPrinciples/bskPrinciples-5.rst

@@ -63,121 +63,54 @@ Next, the simulation stop time is extended for an additional 10s to 20s total an
 Retaining Message Objects in Memory
 -----------------------------------
 
-When creating stand-alone message objects in Python, it's crucial to understand that these objects must be retained in memory to function properly. If a message object is created locally within a function or method and not stored in a persistent variable, Python's garbage collector may remove it from memory once the function exits, even if C++ components still need to access it.
+Before Basilisk 2.12, a stand-alone message created inside a Python helper had to
+be stored in a persistent variable. Otherwise, Python could garbage-collect the
+message after the helper returned while a module still held pointers to its data.
 
-This is particularly important when:
+Starting with Basilisk 2.12, object-based message subscriptions automatically
+retain their source for the lifetime of the subscription. A stand-alone message
+may therefore leave Python scope safely while an input message remains subscribed
+to it. This applies whether the input reader is embedded in a C or C++ module. If
+the source is embedded in another module or C-module config, Basilisk retains that
+owning object rather than the temporary Python message proxy.
 
-1. Creating message objects in subroutines or helper functions
-2. Dynamically generating messages based on runtime conditions
-3. Setting up message interfaces that will be used throughout simulation
+Message recorders follow the same lifetime rule. A recorder created from a
+stand-alone or module-embedded message retains the object that owns the message
+storage until the recorder is released.
 
-**Example: Using a Class-Level Registry to Retain Objects**
-
-A common pattern in Basilisk to ensure message objects remain in memory is to use a
-class-level registry or list that persists for the lifetime of your simulation.
-Here's how this is implemented in Basilisk's own code:
-
-.. code-block:: python
-
-    class BskSimulation:
-        def __init__(self):
-            # Create class-level registry if it doesn't exist
-            if not hasattr(self, '_message_registry'):
-                self._message_registry = []
-
-        def setup_sensors(self):
-            """Set up sensor messages and devices."""
-
-            # Create a message
-            sensor_msg = messaging.SensorMsg()
-
-            # Store message in class-level registry to prevent garbage collection
-            self._message_registry.append(sensor_msg)
-
-            # Create the sensor module that will use this message
-            self.sensor_module = sensorModule.SensorModule()
-            self.sensor_module.sensorOutMsg.subscribeTo(sensor_msg)
-
-            return
-
-
-This pattern is used in Basilisk's own implementation for components like Coarse
-Sun Sensors (CSS):
+For example, this helper does not need to return or otherwise retain ``inputMsg``:
 
 .. code-block:: python
 
-    def SetCSSConstellation(self):
-        """Set the CSS sensors"""
-        self.CSSConstellationObject.ModelTag = "cssConstellation"
-
-        # Create class-level registry if it doesn't exist
-        if not hasattr(self, '_css_registry'):
-            self._css_registry = []
+    def connectInput(module):
+        """Create and connect a stand-alone input message."""
+        payload = messaging.CModuleTemplateMsgPayload(dataVector=[1.0, 2.0, 3.0])
+        inputMsg = messaging.CModuleTemplateMsg().write(payload)
+        module.dataInMsg.subscribeTo(inputMsg)
 
-        def setupCSS(cssDevice):
-            cssDevice = coarseSunSensor.CoarseSunSensor()
-            cssDevice.fov = 80. * mc.D2R
-            cssDevice.scaleFactor = 2.0
-            cssDevice.sunInMsg.subscribeTo(self.gravFactory.spiceObject.planetStateOutMsgs[self.sun])
-            cssDevice.stateInMsg.subscribeTo(self.scObject.scStateOutMsg)
-            # Store CSS in class-level registry to prevent garbage collection
-            self._css_registry.append(cssDevice)
 
-        # Create CSS devices and add them to the registry...
-
-**Alternative Approach: Using a Dictionary Registry**
-
-For more complex simulations where you need to retrieve specific messages later,
-you can use a dictionary-based registry:
+Keeping an explicit Python reference remains valid and can make ownership clearer.
+It is also useful when the caller needs to write new data, inspect the message,
+unsubscribe and reconnect it, or otherwise access it later:
 
 .. code-block:: python
 
     class BskSimulation:
-        """A registry to keep message objects alive in Python memory."""
-
         def __init__(self):
-            self.messages = {}
-
-        def make_message(self, name):
-            """Make a message object with a unique name."""
-            msg_obj = messaging.SensorMsg()
-            self.messages[name] = msg_obj
-            return msg_obj
-
-        def get_message(self, name):
-            """Retrieve a message object by name."""
-            return self.messages.get(name)
-
-**Have Parent Method Retain the Msg Object in memory**
-
-If the message setup method returns in instance of the message, then the parent method
-could be responsible for retaining this message object in memory
-
-.. code-block:: python
-
-    class BskSimulation:
-        """A registry to keep message objects alive in Python memory."""
-
-        def make_message(self):
-            """Make a message object and return it."""
-            msg = messaging.SensorMsg()
-            msg.fov = 2.0
-            return msg
-
-        def parent_method(self, name):
-            """Retrieve a message object by name."""
-            self.sensorMsg = self.make_message()
-            return
-
+            self.inputMsg = messaging.CModuleTemplateMsg()
 
-**Common Pitfalls**
+        def connectInput(self, module, payload):
+            """Write and connect the retained input message."""
+            self.inputMsg.write(payload)
+            module.dataInMsg.subscribeTo(self.inputMsg)
 
-Without proper retention, you might encounter issues like:
 
-- Messages that appear to be properly connected but don't transmit data
-- Simulation components that can't communicate as expected
-- Mysterious segmentation faults or access violations in C++ code
+The automatic lifetime guarantee has the following boundaries:
 
-By using a registry pattern or ensuring message objects are stored in long-lived
-variables, you can avoid these issues and create more modular, maintainable
-simulation code.
+* Calling ``unsubscribe()`` releases the subscription's reference. Retain the
+  message explicitly if it must be available for a later reconnection.
+* Subscribing by a raw integer address is caller-owned and does not retain a
+  Python source object.
+* This guarantee applies to message sources connected through ``subscribeTo()``.
+  Modules, sensors, effectors, and other wrapped simulation objects can have
+  separate ownership requirements and may still need persistent Python references.

docs/source/Support/bskReleaseNotesSnippets/1433-cmsg-reader-keepalive.rst

@@ -0,0 +1 @@
+- Fixed use-after-free paths involving Python-owned C messages. A C module input message (a ``Msg_C`` reader) now keeps its subscribed source alive for as long as the subscription is live, and a recorder created from a ``Msg_C`` keeps the stand-alone message or embedded-message owner alive for the recorder's lifetime. Subscription references are released on ``unsubscribe()``, on re-subscribe, or when the subscriber owner is garbage-collected; recorder references are released when the recorder is garbage-collected. This completes the message keep-alive started for the C++ reader and recorder direction in issue #676.

docs/source/conf.py

@@ -415,6 +415,7 @@ def grabRelevantFiles(self,dir_path):
         removeList = []
         for i in range(len(files_in_dir)):
             if "__init__" in files_in_dir[i] or \
+                    (files_in_dir[i].endswith(".py") and os.path.basename(files_in_dir[i]).startswith("_")) or \
                     "conftest.py" in files_in_dir[i] or \
                     "*.xml" in files_in_dir[i] or \
                     "vizMessage.pb.cc" in files_in_dir[i] or \

examples/scenarioFlexiblePanel.py

@@ -131,12 +131,6 @@ def createSimBaseClass():
     fswTimeStep = macros.sec2nano(0.5)
     scSim.fswProcess.addTask(scSim.CreateNewTask(scSim.fswTaskName, fswTimeStep))
 
-    # Keep standalone reference messages alive for the run. They are created in
-    # setup helpers that return before the simulation runs; without a Python
-    # reference their C++ backing is garbage collected and the subscriber reads a
-    # dead message (a fallout of the .disown() removal in #918).
-    scSim.refMessages = []
-
     return scSim
 
 
@@ -430,7 +424,6 @@ def setUpControl(scSim, extFTObject, attError, scGeometry):
 
     mrpControl.guidInMsg.subscribeTo(attError.attGuidOutMsg)
     mrpControl.vehConfigInMsg.subscribeTo(configDataMsg)
-    scSim.refMessages.append(configDataMsg)
     extFTObject.cmdTorqueInMsg.subscribeTo(mrpControl.cmdTorqueOutMsg)
 
 

src/architecture/_GeneralModuleFiles/swig_c_wrap.i

@@ -101,11 +101,22 @@ class CWrapper : public SysModel {
       template <typename T> inline void Reset_ ## functionSuffix(T, uint64_t, int64_t) {}
     %}
 
-    // The constructor CWrapper(TConfig* config) takes ownership of the given pointer
-    // We don't want the Python object for this config to also think it owns the memory
+    // Config and CWrapper constructors register embedded Msg_C storage owners.
+    // When a wrapper takes a config pointer, transferModuleOwner moves existing
+    // keep-alives to the wrapper. Keep these pythonappend bodies free of apostrophes
+    // and hash comment lines because SWIG macro expansion mis-parses them.
+    %pythonappend configName::configName() %{
+        from Basilisk.architecture.messaging import _msgKeepAlive
+        _msgKeepAlive.registerModule(self)
+    %}
+
     %pythonappend CWrapper::CWrapper %{
+        from Basilisk.architecture.messaging import _msgKeepAlive
         if (len(args)) > 0:
             args[0].thisown = False
+            _msgKeepAlive.transferModuleOwner(args[0], self)
+        else:
+            _msgKeepAlive.registerModule(self)
     %}
 
     %include "moduleName.h"

src/architecture/messaging/CMakeLists.txt

@@ -110,6 +110,20 @@ endfunction(generate_messages)
 # TODO: Deprecate this!
 configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cMsgCInterfacePy/__init__.py ${CMAKE_BINARY_DIR}/Basilisk/architecture/cMsgCInterfacePy/__init__.py COPYONLY)
 
+# issue #1433: Python-side keep-alive helpers for Msg_C subscribers. Copy at
+# build time so this helper does not make the messaging package directory exist
+# during configure, before generatePackageInit.py writes messaging/__init__.py.
+set(MSG_KEEP_ALIVE_PY "${CMAKE_BINARY_DIR}/Basilisk/architecture/messaging/_msgKeepAlive.py")
+add_custom_command(
+  OUTPUT ${MSG_KEEP_ALIVE_PY}
+  COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_BINARY_DIR}/Basilisk/architecture/messaging"
+  COMMAND ${CMAKE_COMMAND} -E copy_if_different
+          "${CMAKE_CURRENT_SOURCE_DIR}/_msgKeepAlive.py"
+          "${MSG_KEEP_ALIVE_PY}"
+  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/_msgKeepAlive.py
+  VERBATIM)
+add_custom_target(msgKeepAlivePy DEPENDS ${MSG_KEEP_ALIVE_PY})
+
 # Track payload headers so the generated Python message package is rebuilt when message definitions change.
 file(GLOB_RECURSE package_init_payload_files CONFIGURE_DEPENDS
   "${CMAKE_CURRENT_SOURCE_DIR}/../msgPayloadDefC/*Payload.h"
@@ -142,6 +156,7 @@ endif(NOT "${EXTERNAL_MODULES_PATH}" STREQUAL "")
 
 # Custom target for establishing dependency
 add_custom_target(swigtrick DEPENDS ${CMAKE_BINARY_DIR}/Basilisk/architecture/messaging/__init__.py)
+add_dependencies(swigtrick msgKeepAlivePy)
 
 # Generate per-payload equality headers. Each "<Payload>_equality.h" is included
 # only by that payload's own SWIG module.