Fix statistics writers sometimes uses the user's publisher [12391]

src/cpp/fastdds/domain/DomainParticipantImpl.cpp

@@ -472,6 +472,15 @@ Publisher* DomainParticipantImpl::create_publisher(
         const PublisherQos& qos,
         PublisherListener* listener,
         const StatusMask& mask)
+{
+    return create_publisher(qos, nullptr, listener, mask);
+}
+
+Publisher* DomainParticipantImpl::create_publisher(
+        const PublisherQos& qos,
+        PublisherImpl** impl,
+        PublisherListener* listener,
+        const StatusMask& mask)
 {
     if (!PublisherImpl::check_qos(qos))
     {
@@ -507,6 +516,11 @@ Publisher* DomainParticipantImpl::create_publisher(
         (void)ret_publisher_enable;
     }
 
+    if (impl)
+    {
+        *impl = pubimpl;
+    }
+
     return pub;
 }
 

src/cpp/fastdds/domain/DomainParticipantImpl.hpp

@@ -119,6 +119,20 @@ class DomainParticipantImpl
             PublisherListener* listener = nullptr,
             const StatusMask& mask = StatusMask::all());
 
+    /**
+     * Create a Publisher in this Participant.
+     * @param qos QoS of the Publisher.
+     * @param[out] impl Return a pointer to the created Publisher's implementation.
+     * @param listenerer Pointer to the listener.
+     * @param mask StatusMask
+     * @return Pointer to the created Publisher.
+     */
+    Publisher* create_publisher(
+            const PublisherQos& qos,
+            PublisherImpl** impl,
+            PublisherListener* listener = nullptr,
+            const StatusMask& mask = StatusMask::all());
+
     /**
      * Create a Publisher in this Participant.
      * @param profile_name Publisher profile name.

src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp

@@ -226,12 +226,7 @@ efd::PublisherImpl* DomainParticipantImpl::create_publisher_impl(
         const efd::PublisherQos& qos,
         efd::PublisherListener* listener)
 {
-    auto impl = new PublisherImpl(this, qos, listener, statistics_listener_);
-    if (nullptr == builtin_publisher_impl_)
-    {
-        builtin_publisher_impl_ = impl;
-    }
-    return impl;
+    return new PublisherImpl(this, qos, listener, statistics_listener_);
 }
 
 efd::SubscriberImpl* DomainParticipantImpl::create_subscriber_impl(
@@ -243,8 +238,13 @@ efd::SubscriberImpl* DomainParticipantImpl::create_subscriber_impl(
 
 void DomainParticipantImpl::create_statistics_builtin_entities()
 {
+    efd::PublisherImpl* builtin_publisher_impl = nullptr;
+
     // Builtin publisher
-    builtin_publisher_ = create_publisher(efd::PUBLISHER_QOS_DEFAULT);
+    builtin_publisher_ = create_publisher(efd::PUBLISHER_QOS_DEFAULT, &builtin_publisher_impl);
+
+    builtin_publisher_impl_ = dynamic_cast<PublisherImpl*>(builtin_publisher_impl);
+    assert(nullptr != builtin_publisher_impl_);
 
     // Enable statistics datawriters
     // 1. Find fastdds_statistics PropertyPolicyQos

test/blackbox/common/DDSBlackboxTestsStatistics.cpp

@@ -30,6 +30,8 @@
 #include <fastdds/dds/core/LoanableSequence.hpp>
 
 #include <fastdds/dds/domain/DomainParticipant.hpp>
+#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
+#include <fastdds/dds/domain/qos/DomainParticipantFactoryQos.hpp>
 #include <fastdds/dds/subscriber/DataReader.hpp>
 #include <fastdds/dds/subscriber/Subscriber.hpp>
 #include <fastdds/dds/topic/TopicDescription.hpp>
@@ -59,8 +61,10 @@ static DataReader* enable_statistics(
         Subscriber* subscriber,
         const std::string& topic_name)
 {
+    auto qos = statistics::dds::STATISTICS_DATAWRITER_QOS;
+    qos.history().depth = 10;
     EXPECT_EQ(ReturnCode_t::RETCODE_OK, participant->enable_statistics_datawriter(
-                topic_name, statistics::dds::STATISTICS_DATAWRITER_QOS));
+                topic_name, qos));
 
     auto topic_desc = participant->lookup_topicdescription(topic_name);
     EXPECT_NE(nullptr, topic_desc);
@@ -168,7 +172,7 @@ TEST(DDSStatistics, simple_statistics_datareaders)
         {"RTPS_SENT_TOPIC",                 statistics::RTPS_SENT_TOPIC,                num_samples},
         {"NETWORK_LATENCY_TOPIC",           statistics::NETWORK_LATENCY_TOPIC,          num_samples},
         {"PUBLICATION_THROUGHPUT_TOPIC",    statistics::PUBLICATION_THROUGHPUT_TOPIC,   num_samples},
-        {"HEARTBEAT_COUNT_TOPIC",           statistics::HEARTBEAT_COUNT_TOPIC,          num_samples},
+        {"HEARTBEAT_COUNT_TOPIC",           statistics::HEARTBEAT_COUNT_TOPIC,          1},
         {"SAMPLE_DATAS_TOPIC",              statistics::SAMPLE_DATAS_TOPIC,             num_samples},
         {"DISCOVERY_TOPIC",                 statistics::DISCOVERY_TOPIC,                1},
         {"PDP_PACKETS_TOPIC",               statistics::PDP_PACKETS_TOPIC,              1},
@@ -296,3 +300,66 @@ TEST(DDSStatistics, simple_statistics_second_writer)
 
 #endif // FASTDDS_STATISTICS
 }
+
+// Regression test for #12390. Mostly a replication of test simple_statistics_second_writer but creating
+// and additional publisher with partitions before the creation of the builtin publisher
+TEST(DDSStatistics, statistics_with_partition_on_user)
+{
+#ifdef FASTDDS_STATISTICS
+    auto domain_id = GET_PID() % 100;
+
+    DomainParticipantQos p_qos = PARTICIPANT_QOS_DEFAULT;
+    DomainParticipantFactory* participant_factory = DomainParticipantFactory::get_instance();
+
+    // We disable the auto-enabling so the builtin entities do not get created.
+    DomainParticipantFactoryQos factory_qos;
+    participant_factory->get_qos(factory_qos);
+    factory_qos.entity_factory().autoenable_created_entities = false;
+    participant_factory->set_qos(factory_qos);
+
+    DomainParticipant* p1 = participant_factory->create_participant(domain_id, p_qos);
+    DomainParticipant* p2 = participant_factory->create_participant(domain_id, p_qos);
+
+    ASSERT_NE(nullptr, p1);
+    ASSERT_NE(nullptr, p2);
+
+    // Now we create a Publisher with a partition
+    PublisherQos pub_qos = PUBLISHER_QOS_DEFAULT;
+    pub_qos.partition().push_back("partition_a");
+    auto user_pub_1 = p1->create_publisher(pub_qos);
+
+    // We enable the participants
+    ASSERT_EQ(ReturnCode_t::RETCODE_OK, p1->enable());
+    ASSERT_EQ(ReturnCode_t::RETCODE_OK, p2->enable());
+
+    auto statistics_p1 = statistics::dds::DomainParticipant::narrow(p1);
+    auto statistics_p2 = statistics::dds::DomainParticipant::narrow(p2);
+
+    ASSERT_NE(nullptr, statistics_p1);
+    ASSERT_NE(nullptr, statistics_p2);
+
+    auto subscriber_p1 = p1->create_subscriber(SUBSCRIBER_QOS_DEFAULT);
+    auto subscriber_p2 = p2->create_subscriber(SUBSCRIBER_QOS_DEFAULT);
+    ASSERT_NE(nullptr, subscriber_p1);
+    ASSERT_NE(nullptr, subscriber_p2);
+
+    auto physical_data_reader_1 = enable_statistics(statistics_p1, subscriber_p1, statistics::PHYSICAL_DATA_TOPIC);
+    auto physical_data_reader_2 = enable_statistics(statistics_p2, subscriber_p2, statistics::PHYSICAL_DATA_TOPIC);
+    ASSERT_NE(nullptr, physical_data_reader_1);
+    ASSERT_NE(nullptr, physical_data_reader_2);
+
+    wait_statistics(physical_data_reader_1, 2, "PHYSICAL_DATA_TOPIC", 10u);
+    wait_statistics(physical_data_reader_2, 2, "PHYSICAL_DATA_TOPIC", 10u);
+
+    disable_statistics(statistics_p1, subscriber_p1, physical_data_reader_1, statistics::PHYSICAL_DATA_TOPIC);
+    disable_statistics(statistics_p2, subscriber_p2, physical_data_reader_2, statistics::PHYSICAL_DATA_TOPIC);
+
+    p1->delete_publisher(user_pub_1);
+    p2->delete_subscriber(subscriber_p2);
+    p1->delete_subscriber(subscriber_p1);
+
+    participant_factory->delete_participant(p2);
+    participant_factory->delete_participant(p1);
+
+#endif // FASTDDS_STATISTICS
+}

test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp

@@ -139,6 +139,15 @@ class DomainParticipantImpl
 
     Publisher* create_publisher(
             const PublisherQos& qos,
+            PublisherListener* listener,
+            const StatusMask& mask)
+    {
+        return create_publisher(qos, nullptr, listener, mask);
+    }
+
+    Publisher* create_publisher(
+            const PublisherQos& qos,
+            PublisherImpl** impl,
             PublisherListener* listener = nullptr,
             const StatusMask& mask = StatusMask::all())
     {
@@ -149,6 +158,12 @@ class DomainParticipantImpl
         std::lock_guard<std::mutex> lock(mtx_pubs_);
         publishers_[pub] = pubimpl;
         pub->enable();
+
+        if (impl)
+        {
+            *impl = pubimpl;
+        }
+
         return pub;
     }