Fix mutex lock count on PDPListener [11861]

src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp

@@ -125,11 +125,12 @@ void PDPListener::onNewCacheChangeAdded(
             {
                 // Create a new one when not found
                 pdata = parent_pdp_->createParticipantProxyData(temp_participant_data_, writer_guid);
+
+                reader->getMutex().unlock();
+                lock.unlock();
+
                 if (pdata != nullptr)
                 {
-                    reader->getMutex().unlock();
-                    lock.unlock();
-
                     logInfo(RTPS_PDP_DISCOVERY, "New participant "
                             << pdata->m_guid << " at "
                             << "MTTLoc: " << pdata->metatraffic_locators

src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp

@@ -123,22 +123,23 @@ void PDPServerListener::onNewCacheChangeAdded(
         // Deserialize the payload to access the discovery info
         CDRMessage_t msg(change->serializedPayload);
         temp_participant_data_.clear();
+        auto participant_data = temp_participant_data_;
 
-        if (temp_participant_data_.readFromCDRMessage(
+        if (participant_data.readFromCDRMessage(
                     &msg,
                     true,
                     pdp_server()->getRTPSParticipant()->network_factory(),
                     pdp_server()->getRTPSParticipant()->has_shm_transport()))
         {
             /* Check PID_VENDOR_ID */
-            if (temp_participant_data_.m_VendorId != fastrtps::rtps::c_VendorId_eProsima)
+            if (participant_data.m_VendorId != fastrtps::rtps::c_VendorId_eProsima)
             {
                 logInfo(RTPS_PDP_LISTENER,
                         "DATA(p|Up) from different vendor is not supported for Discover-Server operation");
                 return;
             }
 
-            fastrtps::ParameterPropertyList_t properties = temp_participant_data_.m_properties;
+            fastrtps::ParameterPropertyList_t properties = participant_data.m_properties;
 
             /* Check DS_VERSION */
             auto ds_version = std::find_if(
@@ -244,7 +245,7 @@ void PDPServerListener::onNewCacheChangeAdded(
                 if (pdp_server()->discovery_db().update(
                             change.get(),
                             ddb::DiscoveryParticipantChangeData(
-                                temp_participant_data_.metatraffic_locators,
+                                participant_data.metatraffic_locators,
                                 is_client,
                                 is_local)))
                 {
@@ -303,7 +304,7 @@ void PDPServerListener::onNewCacheChangeAdded(
                 logInfo(RTPS_PDP_LISTENER, "Registering a new participant: " << guid);
 
                 // Create a new participant proxy entry
-                pdata = pdp_server()->createParticipantProxyData(temp_participant_data_, writer_guid);
+                pdata = pdp_server()->createParticipantProxyData(participant_data, writer_guid);
                 // Realease PDP mutex
                 lock.unlock();
 
@@ -326,7 +327,7 @@ void PDPServerListener::onNewCacheChangeAdded(
             else
             {
                 // Update proxy
-                pdata->updateData(temp_participant_data_);
+                pdata->updateData(participant_data);
                 pdata->isAlive = true;
                 // Realease PDP mutex
                 lock.unlock();

test/blackbox/CMakeLists.txt

@@ -252,6 +252,8 @@ if(NOT ((MSVC OR MSVC_IDE) AND EPROSIMA_INSTALLER) AND fastcdr_FOUND)
             ${CMAKE_CURRENT_BINARY_DIR}/PubSubWriter.xml)
         configure_file(${CMAKE_CURRENT_SOURCE_DIR}/PubSubReader.xml.in
             ${CMAKE_CURRENT_BINARY_DIR}/PubSubReader.xml)
+        configure_file(${CMAKE_CURRENT_SOURCE_DIR}/discovery_participant_flags.xml
+            ${CMAKE_CURRENT_BINARY_DIR}/discovery_participant_flags.xml)
         configure_file(${CMAKE_CURRENT_SOURCE_DIR}/persistence.xml
             ${CMAKE_CURRENT_BINARY_DIR}/persistence.xml)
         configure_file(${CMAKE_CURRENT_SOURCE_DIR}/StatisticsDomainParticipant.xml

test/blackbox/api/dds-pim/PubSubReader.hpp

@@ -568,13 +568,52 @@ class PubSubReader
         std::cout << "Reader discovery finished..." << std::endl;
     }
 
+    bool wait_participant_discovery(
+            unsigned int min_participants = 1,
+            std::chrono::seconds timeout = std::chrono::seconds::zero())
+    {
+        bool ret_value = true;
+        std::unique_lock<std::mutex> lock(mutexDiscovery_);
+
+        std::cout << "Reader is waiting discovery of at least " << min_participants << " participants..." << std::endl;
+
+        if (timeout == std::chrono::seconds::zero())
+        {
+            cvDiscovery_.wait(lock, [&]()
+                    {
+                        return participant_matched_ >= min_participants;
+                    });
+        }
+        else
+        {
+            if (!cvDiscovery_.wait_for(lock, timeout, [&]()
+                    {
+                        return participant_matched_ >= min_participants;
+                    }))
+            {
+                ret_value = false;
+            }
+        }
+
+        if (ret_value)
+        {
+            std::cout << "Reader participant discovery finished successfully..." << std::endl;
+        }
+        else
+        {
+            std::cout << "Reader participant discovery finished unsuccessfully..." << std::endl;
+        }
+
+        return ret_value;
+    }
+
     bool wait_participant_undiscovery(
             std::chrono::seconds timeout = std::chrono::seconds::zero())
     {
         bool ret_value = true;
         std::unique_lock<std::mutex> lock(mutexDiscovery_);
 
-        std::cout << "Reader is waiting undiscovery..." << std::endl;
+        std::cout << "Reader is waiting participant undiscovery..." << std::endl;
 
         if (timeout == std::chrono::seconds::zero())
         {
@@ -596,11 +635,11 @@ class PubSubReader
 
         if (ret_value)
         {
-            std::cout << "Reader undiscovery finished successfully..." << std::endl;
+            std::cout << "Reader participant undiscovery finished successfully..." << std::endl;
         }
         else
         {
-            std::cout << "Reader undiscovery finished unsuccessfully..." << std::endl;
+            std::cout << "Reader participant undiscovery finished unsuccessfully..." << std::endl;
         }
 
         return ret_value;
@@ -1053,6 +1092,13 @@ class PubSubReader
         return *this;
     }
 
+    PubSubReader& ignore_participant_flags(
+            eprosima::fastrtps::rtps::ParticipantFilteringFlags_t flags)
+    {
+        participant_qos_.wire_protocol().builtin.discovery_config.ignoreParticipantFlags = flags;
+        return *this;
+    }
+
     PubSubReader& socket_buffer_size(
             uint32_t sockerBufferSize)
     {

test/blackbox/common/BlackboxTestsDiscovery.cpp

@@ -1264,4 +1264,4 @@ TEST(Discovery, ServerClientEnvironmentSetUp)
     output.clear();
     ASSERT_FALSE(load_environment_server_info(text, output));
 
-}
+}

test/blackbox/common/DDSBlackboxTestsDiscovery.cpp

@@ -0,0 +1,55 @@
+// Copyright 2020 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <gtest/gtest.h>
+
+#include "BlackboxTests.hpp"
+
+#include "PubSubReader.hpp"
+
+// Regression test for redmine issue 11857
+TEST(DDSDiscovery, IgnoreParticipantFlags)
+{
+    // This participant is created with:
+    // - ignoreParticipantFlags = FILTER_SAME_PROCESS (will avoid discovery of p2)
+    // - metatrafficUnicastLocatorList = 127.0.0.1:7399, 127.0.0.1:7398 (to ensure two listening threads are created)
+    PubSubReader<HelloWorldType> p1(TEST_TOPIC_NAME);
+    p1.set_xml_filename("discovery_participant_flags.xml");
+    p1.set_participant_profile("participant_1");
+    p1.init();
+    EXPECT_TRUE(p1.isInitialized());
+
+    // This participant is created with initialPeersList = 127.0.0.1:7399
+    // When the announcements of this participant arrive to p1, they will be ignored, and thus p1 will not
+    // announce itself back to p2.
+    PubSubReader<HelloWorldType> p2(TEST_TOPIC_NAME);
+    p2.set_xml_filename("discovery_participant_flags.xml");
+    p2.set_participant_profile("participant_2");
+    p2.init();
+    EXPECT_TRUE(p2.isInitialized());
+    EXPECT_FALSE(p2.wait_participant_discovery(1, std::chrono::seconds(1)));
+    EXPECT_FALSE(p1.wait_participant_discovery(1, std::chrono::seconds(1)));
+
+    // This participant is created with:
+    // - initialPeersList = 127.0.0.1:7398
+    // - a custom guid prefix
+    // The announcements of this participant will arrive to p1 on a different listening thread.
+    // Due to the custom prefix, they should not be ignored, and mutual discovery should happen
+    PubSubReader<HelloWorldType> p3(TEST_TOPIC_NAME);
+    p3.set_xml_filename("discovery_participant_flags.xml");
+    p3.set_participant_profile("participant_3");
+    p3.init();
+    EXPECT_TRUE(p1.wait_participant_discovery());
+    EXPECT_TRUE(p3.wait_participant_discovery());
+}

test/blackbox/discovery_participant_flags.xml

@@ -0,0 +1,67 @@
+<?xml version="1.0" encoding="utf-8"  ?>
+<dds xmlns="http://www.eprosima.com/XMLSchemas/fastRTPS_Profiles">
+    <profiles>          
+
+        <participant profile_name="participant_1">
+            <rtps>
+                <name>Discovery.IgnoreParticipantFlags.p1</name>
+
+                <builtin>
+                    <discovery_config>
+                        <ignoreParticipantFlags>FILTER_SAME_PROCESS</ignoreParticipantFlags>
+                    </discovery_config>
+                    <metatrafficUnicastLocatorList>
+                        <locator>
+                            <udpv4>
+                                <port>7399</port>
+                                <address>127.0.0.1</address>
+                            </udpv4>
+                        </locator>
+                        <locator>
+                            <udpv4>
+                                <port>7398</port>
+                                <address>127.0.0.1</address>
+                            </udpv4>
+                        </locator>
+                    </metatrafficUnicastLocatorList>
+                </builtin>
+            </rtps>
+        </participant>
+
+        <participant profile_name="participant_2">
+            <rtps>
+                <name>Discovery.IgnoreParticipantFlags.p2</name>
+
+                <builtin>
+                    <initialPeersList>
+                        <locator>
+                            <udpv4>
+                                <port>7399</port>
+                                <address>127.0.0.1</address>
+                            </udpv4>
+                        </locator>
+                    </initialPeersList>
+                </builtin>
+            </rtps>
+        </participant>
+
+        <participant profile_name="participant_3">
+            <rtps>
+                <prefix>f0.f0.f0.f0.f0.f0.f0.f0.f0.f0.f0.f0</prefix>
+                <name>Discovery.IgnoreParticipantFlags.p3</name>
+
+                <builtin>
+                    <initialPeersList>
+                        <locator>
+                            <udpv4>
+                                <port>7398</port>
+                                <address>127.0.0.1</address>
+                            </udpv4>
+                        </locator>
+                    </initialPeersList>
+                </builtin>
+            </rtps>
+        </participant>
+
+    </profiles>
+</dds>