Fix ASAN errors in API tests [15965]

.github/workflows/asan.yml

@@ -5,7 +5,7 @@ on:
   pull_request:
   push:
     branches:
-      - main
+      - master
   schedule:
     - cron: '0 1 * * *'
 

include/fastdds/rtps/participant/ParticipantDiscoveryInfo.h

@@ -28,9 +28,9 @@ namespace fastrtps {
 namespace rtps {
 
 /**
-* Class ParticipantDiscoveryInfo with discovery information of the Participant.
-* @ingroup RTPS_MODULE
-*/
+ * Class ParticipantDiscoveryInfo with discovery information of the Participant.
+ * @ingroup RTPS_MODULE
+ */
 struct ParticipantDiscoveryInfo
 {
     //!Enum DISCOVERY_STATUS, four different status for discovered participants.
@@ -39,25 +39,34 @@ struct ParticipantDiscoveryInfo
     enum RTPS_DllAPI DISCOVERY_STATUS
 #else
     enum DISCOVERY_STATUS
-#endif
+#endif // if defined(_WIN32)
     {
         DISCOVERED_PARTICIPANT,
         CHANGED_QOS_PARTICIPANT,
         REMOVED_PARTICIPANT,
         DROPPED_PARTICIPANT
     };
 
-    ParticipantDiscoveryInfo(const ParticipantProxyData& data)
+    ParticipantDiscoveryInfo(
+            const ParticipantProxyData& data)
         : status(DISCOVERED_PARTICIPANT)
         , info(data)
-    {}
+    {
+    }
 
-    virtual ~ParticipantDiscoveryInfo() {}
+    virtual ~ParticipantDiscoveryInfo()
+    {
+    }
 
     //! Status
     DISCOVERY_STATUS status;
 
-    //! Participant discovery info
+    /**
+     * @brief Participant discovery info
+     *
+     * @todo This is a reference to an object that could be deleted, thus it should not be a reference
+     * (intraprocess case -> BlackboxTests_DDS_PIM.DDSDiscovery.ParticipantProxyPhysicalData).
+     */
     const ParticipantProxyData& info;
 };
 
@@ -70,9 +79,14 @@ struct ParticipantAuthenticationInfo
         UNAUTHORIZED_PARTICIPANT
     };
 
-    ParticipantAuthenticationInfo() : status(UNAUTHORIZED_PARTICIPANT) {}
+    ParticipantAuthenticationInfo()
+        : status(UNAUTHORIZED_PARTICIPANT)
+    {
+    }
 
-    ~ParticipantAuthenticationInfo() {}
+    ~ParticipantAuthenticationInfo()
+    {
+    }
 
     //! Status
     AUTHENTICATION_STATUS status;
@@ -81,15 +95,18 @@ struct ParticipantAuthenticationInfo
     GUID_t guid;
 };
 
-inline bool operator==(const ParticipantAuthenticationInfo& l, const ParticipantAuthenticationInfo& r)
+inline bool operator ==(
+        const ParticipantAuthenticationInfo& l,
+        const ParticipantAuthenticationInfo& r)
 {
     return l.status == r.status &&
-        l.guid == r.guid;
+           l.guid == r.guid;
 }
-#endif
 
-}
-}
-}
+#endif // if HAVE_SECURITY
+
+} // namespace rtps
+} // namespace fastrtps
+} // namespace eprosima
 
 #endif // _FASTDDS_RTPS_PARTICIPANT_PARTICIPANTDISCOVERYINFO_H__

include/fastdds/rtps/transport/ChainingTransport.h

@@ -16,6 +16,7 @@
 #define _FASTDDS_RTPS_TRANSPORT_CHAININGTRANSPORT_H_
 
 #include <map>
+#include <memory>
 
 #include "TransportInterface.h"
 #include "ChainingTransportDescriptor.h"
@@ -26,6 +27,22 @@ namespace rtps {
 
 class ChainingReceiverResource;
 
+/**
+ * @brief Deleter for a ChainingReceiverResource
+ *
+ * @note this is required to create a \c unique_ptr of a \c ChainingReceiverResource as it is
+ * an incomplete class for this header.
+ */
+struct ChainingReceiverResourceDeleter
+{
+    void operator ()(
+            ChainingReceiverResource* p);
+};
+
+//! Type of the \c unique_ptr of a \c ChainingReceiverResource .
+using ChainingReceiverResourceReferenceType =
+        std::unique_ptr<ChainingReceiverResource, ChainingReceiverResourceDeleter>;
+
 /**
  * This is the base class for chaining adapter transports.
  *    - Directly proxies all operations except Send and Receive
@@ -340,7 +357,7 @@ class ChainingTransport : public TransportInterface
 
 private:
 
-    std::map<fastrtps::rtps::Locator_t, ChainingReceiverResource*> receiver_resources_;
+    std::map<fastrtps::rtps::Locator_t, ChainingReceiverResourceReferenceType> receiver_resources_;
 };
 
 } // namespace rtps

src/cpp/rtps/transport/ChainingTransport.cpp

@@ -20,6 +20,12 @@ namespace eprosima {
 namespace fastdds {
 namespace rtps {
 
+void ChainingReceiverResourceDeleter::operator ()(
+        ChainingReceiverResource* p)
+{
+    delete p;
+}
+
 bool ChainingTransport::OpenInputChannel(
         const fastrtps::rtps::Locator_t& loc,
         TransportReceiverInterface* receiver_interface,

test/blackbox/common/BlackboxTests.cpp

@@ -79,7 +79,7 @@ class BlackboxEnvironment : public ::testing::Environment
     {
         //Log::Reset();
         eprosima::fastdds::dds::Log::KillThread();
-        eprosima::fastrtps::rtps::RTPSDomain::stopAll();
+        // Please, do not remove RTPSDomain before DomainParticipantFactory
     }
 
 };

test/blackbox/common/DDSBlackboxTestsDiscovery.cpp

@@ -423,14 +423,18 @@ TEST(DDSDiscovery, ParticipantProxyPhysicalData)
                 ParticipantDiscoveryInfo&& info)
         {
             std::unique_lock<std::mutex> lck(*mtx_);
-            static_cast<void>(participant);
-            if (nullptr != remote_participant_info)
+            if (info.status ==
+                    eprosima::fastrtps::rtps::ParticipantDiscoveryInfo::DISCOVERY_STATUS::DISCOVERED_PARTICIPANT)
             {
-                delete remote_participant_info;
+                static_cast<void>(participant);
+                if (nullptr != remote_participant_info)
+                {
+                    delete remote_participant_info;
+                }
+                remote_participant_info = new ParticipantDiscoveryInfo(info);
+                found_->store(true);
+                cv_->notify_one();
             }
-            remote_participant_info = new ParticipantDiscoveryInfo(info);
-            found_->store(true);
-            cv_->notify_one();
         }
 
         ParticipantDiscoveryInfo* remote_participant_info;

test/unittest/dds/participant/CMakeLists.txt

@@ -16,10 +16,12 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/../profiles/test_xml_profiles.xml
     ${CMAKE_CURRENT_BINARY_DIR}/test_xml_profiles.xml
     COPYONLY)
 
+# NOTE: having here ServerAttributes.cpp is dangerous as this .cpp is already compiled.
+# However, I found no way to do it work, so here it stays.
 set(PARTICIPANTTESTS_SOURCE
-    ParticipantTests.cpp
-    ${PROJECT_SOURCE_DIR}/src/cpp/rtps/builtin/discovery/participant/ServerAttributes.cpp
-    ${PROJECT_SOURCE_DIR}/src/cpp/utils/SystemInfo.cpp)
+        ParticipantTests.cpp
+        ${PROJECT_SOURCE_DIR}/src/cpp/rtps/builtin/discovery/participant/ServerAttributes.cpp
+    )
 
 if(WIN32)
     add_definitions(-D_WIN32_WINNT=0x0601)

test/unittest/dds/participant/ParticipantTests.cpp

@@ -274,6 +274,19 @@ class BarType
     std::array<char, 256> message_;
 };
 
+// NOTE: This function is duplicated from SystemInfo because it is not in the API and could not be added to test
+// compilation as that file is already compiled and linked, and doing such thing is wrong and would make a kitten cry.
+// (it duplicates an instantiated variable 'environment_file_' and so provoke a double free).
+int process_id()
+{
+#if defined(__cplusplus_winrt)
+    return (int)GetCurrentProcessId();
+#elif defined(_WIN32)
+    return (int)_getpid();
+#else
+    return (int)getpid();
+#endif // platform selection
+}
 
 TEST(ParticipantTests, DomainParticipantFactoryGetInstance)
 {
@@ -721,7 +734,7 @@ void set_environment_file(
 std::string get_environment_filename()
 {
     std::ostringstream name;
-    name << "environment_file_" << SystemInfo::instance().process_id() << ".json";
+    name << "environment_file_" << process_id() << ".json";
     std::string fname = name.str();
     // 'touch' the file
     std::ofstream f(fname);