676 subscribed messages which go out of scope get garbage collected and cause undefined behaviour

docs/source/Support/bskReleaseNotes.rst

@@ -146,6 +146,8 @@ Version  2.6.0  (Feb. 21, 2025)
 - Download ``cspice`` using ``conan`` instead of providing custom libraries.  This ensures all platforms are using
   the same version of ``cspice``.
 - Ensured that the ability to designate an external BSK folder still works with ``conan2``
+- Added a fix for the message subscription bug in which the message data was not retained after going out of scope.
+- Added unit test for message subscription and message data retention after going out of scope to ``test_messaging.py``.
 
 
 Version 2.5.0 (Sept. 30, 2024)

src/architecture/_GeneralModuleFiles/swig_c_wrap.i

@@ -23,6 +23,8 @@
     #include <architecture/utilities/bskLogging.h>
     #include <memory>
     #include <type_traits>
+    #include <vector>
+    #include <functional>
 %}
 %include <std_string.i>
 
@@ -63,6 +65,22 @@ class CWrapper : public SysModel {
     std::unique_ptr<TConfig> config; //!< class variable
 };
 
+class CModuleWrapper {
+private:
+    std::vector<std::function<void()>> destructionCallbacks;
+
+public:
+    void addDestructionCallback(std::function<void()> callback) {
+        destructionCallbacks.push_back(callback);
+    }
+
+    ~CModuleWrapper() {
+        for (auto& callback : destructionCallbacks) {
+            callback();
+        }
+    }
+};
+
 %}
 
 %define %c_wrap_3(moduleName, configName, functionSuffix)
@@ -83,7 +101,7 @@ class CWrapper : public SysModel {
     in overload resolution than methods using the explicit type.
 
     This means that:
-    
+
     Reset_hillPoint(hillPointConfig*, uint64_t, int64_t) { ... }
 
     will always be chosen before:
@@ -103,7 +121,7 @@ class CWrapper : public SysModel {
         if (len(args)) > 0:
             args[0].thisown = False
     %}
-    
+
     %include "moduleName.h"
 
     %template(moduleName) CWrapper<configName,Update_ ## functionSuffix,SelfInit_ ## functionSuffix,Reset_ ## functionSuffix>;

src/architecture/messaging/messaging.h

@@ -23,6 +23,7 @@ ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 #include "architecture/utilities/bskLogging.h"
 #include <typeinfo>
 #include <stdlib.h>
+#include <functional>
 
 /*! forward-declare sim message for use by read functor */
 template<typename messageType>
@@ -38,6 +39,8 @@ class ReadFunctor{
     messageType* payloadPointer;    //!< -- pointer to the incoming msg data
     MsgHeader *headerPointer;      //!< -- pointer to the incoming msg header
     bool initialized;               //!< -- flag indicating if the input message is connect to another message
+    std::function<void()> refIncCallback;
+    std::function<void()> refDecCallback;
 
 public:
     //!< -- BSK Logging
@@ -46,10 +49,18 @@ class ReadFunctor{
 
 
     //! constructor
-    ReadFunctor() : initialized(false) {};
+    ReadFunctor() :
+        initialized(false),
+        refIncCallback([](){}),
+        refDecCallback([](){}) {}
 
     //! constructor
-    ReadFunctor(messageType* payloadPtr, MsgHeader *headerPtr) : payloadPointer(payloadPtr), headerPointer(headerPtr), initialized(true){};
+    ReadFunctor(messageType* payloadPtr, MsgHeader *headerPtr) :
+        payloadPointer(payloadPtr),
+        headerPointer(headerPtr),
+        initialized(true),
+        refIncCallback([](){}),
+        refDecCallback([](){}) {}
 
     //! constructor
     const messageType& operator()(){
@@ -174,6 +185,42 @@ class ReadFunctor{
 
     //! Recorder method description
     Recorder<messageType> recorder(uint64_t timeDiff = 0){return Recorder<messageType>(this, timeDiff);}
+
+    /**
+     * @brief Set the reference counting callbacks for the source message
+     * @param incRef Function to increment source reference count
+     * @param decRef Function to decrement source reference count
+     */
+    void setSourceRef(std::function<void()> incRef, std::function<void()> decRef) {
+        if (refDecCallback) {
+            refDecCallback();
+        }
+        refIncCallback = incRef;
+        refDecCallback = decRef;
+        if (refIncCallback) {
+            refIncCallback();
+        }
+    }
+
+    /*! Check if this reader is subscribed to a specific message source
+     * @param source Pointer to message source to check
+     * @return 1 if subscribed, 0 if not
+     */
+    uint8_t isSubscribedTo(const Message<messageType>* source) const {
+        if (!source) return 0;
+        return (this->payloadPointer == source->getPayloadPtr() &&
+                this->headerPointer == source->getHeaderPtr());
+    }
+
+    /*! Check if this reader is subscribed to a void pointer source
+     * @param source Void pointer to message source to check
+     * @return 1 if subscribed, 0 if not
+     */
+    uint8_t isSubscribedTo(const void* source) const {
+        const Message<messageType>* msgSource =
+            static_cast<const Message<messageType>*>(source);
+        return isSubscribedTo(msgSource);
+    }
 };
 
 /*! Write Functor */
@@ -232,6 +279,16 @@ class Message{
 
     //! Return the memory size of the payload, be careful about dynamically sized things
     uint64_t getPayloadSize() {return sizeof(messageType);};
+
+    /*! Get pointer to message payload
+     * @return Const pointer to payload data
+     */
+    const messageType* getPayloadPtr() const { return &payload; }
+
+    /*! Get pointer to message header
+     * @return Const pointer to message header
+     */
+    const MsgHeader* getHeaderPtr() const { return &header; }
 };
 
 

src/architecture/messaging/newMessaging.ih

@@ -34,38 +34,50 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 %template(messageType ## Reader) ReadFunctor<messageTypePayload>;
 %extend ReadFunctor<messageTypePayload> {
-        %pythoncode %{
-            def subscribeTo(self, source):
-                if type(source) == messageType:
-                    self.__subscribe_to(source)
+    void setPySourceRef(PyObject* source) {
+        if (source) {
+            self->setSourceRef(
+                [source]() { Py_INCREF(source); },
+                [source]() { Py_DECREF(source); }
+            );
+        }
+    }
+
+    %pythoncode %{
+        def subscribeTo(self, source):
+            if type(source) == messageType:
+                self.__subscribe_to(source)
+                self.setPySourceRef(source)
+                return
+
+            try:
+                from Basilisk.architecture.messaging.messageType ## Payload import messageType ## _C
+                if type(source) == messageType ## _C:
+                    self.__subscribe_to_C(source)
+                    self.setPySourceRef(source)
                     return
-
-                try:
-                    from Basilisk.architecture.messaging.messageType ## Payload import messageType ## _C
-                    if type(source) == messageType ## _C:
-                        self.__subscribe_to_C(source)
-                        return
-                except ImportError:
-                    pass
-
-                raise Exception('tried to subscribe ReadFunctor<messageTypePayload> to output message type'
-                                + str(type(source)))
-
-
-            def isSubscribedTo(self, source):
-                if type(source) == messageType:
-                    return self.__is_subscribed_to(source)
-
-                try:
-                    from Basilisk.architecture.messaging.messageType ## Payload import messageType ## _C
-                except ImportError:
-                    return 0
-
+            except ImportError:
+                pass
+
+            raise Exception('tried to subscribe ReadFunctor<messageTypePayload> to output message type'
+                            + str(type(source)))
+    %}
+
+    %pythoncode %{
+        def isSubscribedTo(self, source):
+            """Check if this reader is subscribed to the given source"""
+            if type(source) == messageType:
+                return self.__is_subscribed_to(source)
+
+            try:
+                from Basilisk.architecture.messaging.messageType ## Payload import messageType ## _C
                 if type(source) == messageType ## _C:
                     return self.__is_subscribed_to_C(source)
-                else:
-                    return 0
-        %}
+            except ImportError:
+                pass
+
+            return 0
+    %}
 };
 
 %template(messageType ## Writer) WriteFunctor<messageTypePayload>;
@@ -110,7 +122,7 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
                         for el, k in zip(attr, range(len(attr))):
                             self.explore_and_find_subattr(el, attr_name + "[" + str(k) + "]", content)
                     else:
-                        # The attribute is a list of common types 
+                        # The attribute is a list of common types
                         content[attr_name] = attr
             elif "Basilisk" in str(type(attr)):
                 # The attribute is a swigged BSK object