Skip to content

Commit

Permalink
Enforce SHM ports open mode exclusions (#4635) (#4646)
Browse files Browse the repository at this point in the history
* Enforce SHM ports open mode exclusions (#4635)

* Refs #20701. Added unit regression tests.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #20701. Added blackbox regression test.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Refs #20701. Fix issue.

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
(cherry picked from commit 3d159dc)

# Conflicts:
#	test/blackbox/common/BlackboxTestsTransportSHM.cpp

* Fix conflicts

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Fix build on unit test

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

* Fix build on unit test

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>

---------

Signed-off-by: Miguel Company <miguelcompany@eprosima.com>
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
  • Loading branch information
mergify[bot] and MiguelCompany authored Apr 12, 2024
1 parent dbdc7a9 commit 54090bf
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 1 deletion.
29 changes: 28 additions & 1 deletion src/cpp/rtps/transport/shared_mem/SharedMemGlobal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -898,12 +898,22 @@ class SharedMemGlobal

void lock_read_exclusive()
{
if (OpenMode::ReadShared == open_mode())
{
throw std::runtime_error("port is opened ReadShared");
}

std::string lock_name = std::string(node_->domain_name) + "_port" + std::to_string(node_->port_id) + "_el";
read_exclusive_lock_ = std::unique_ptr<RobustExclusiveLock>(new RobustExclusiveLock(lock_name));
}

void lock_read_shared()
{
if (OpenMode::ReadExclusive == open_mode())
{
throw std::runtime_error("port is opened ReadExclusive");
}

std::string lock_name = std::string(node_->domain_name) + "_port" + std::to_string(node_->port_id) + "_sl";
read_shared_lock_ = std::unique_ptr<RobustSharedLock>(new RobustSharedLock(lock_name));
}
Expand Down Expand Up @@ -1124,7 +1134,24 @@ class SharedMemGlobal
std::stringstream ss;

ss << port_node->port_id << " (" << port_node->uuid.to_string() <<
") because is ReadExclusive locked";
") because it was already locked";

err_reason = ss.str();
port.reset();
}
}
else if (open_mode == Port::OpenMode::ReadShared)
{
try
{
port->lock_read_shared();
}
catch (const std::exception&)
{
std::stringstream ss;

ss << port_node->port_id << " (" << port_node->uuid.to_string() <<
") because it had a ReadExclusive lock";

err_reason = ss.str();
port.reset();
Expand Down
55 changes: 55 additions & 0 deletions test/blackbox/common/BlackboxTestsTransportSHM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "BlackboxTests.hpp"

#include "PubSubParticipant.hpp"
#include "PubSubReader.hpp"
#include "PubSubWriter.hpp"

Expand All @@ -28,6 +29,8 @@ using namespace eprosima::fastrtps;

using SharedMemTransportDescriptor = eprosima::fastdds::rtps::SharedMemTransportDescriptor;
using test_SharedMemTransportDescriptor = eprosima::fastdds::rtps::test_SharedMemTransportDescriptor;
using Locator = eprosima::fastdds::rtps::Locator;
using LocatorList = eprosima::fastdds::rtps::LocatorList;

TEST(SHM, TransportPubSub)
{
Expand Down Expand Up @@ -70,6 +73,58 @@ TEST(SHM, TransportPubSub)
reader.wait_participant_undiscovery();
}

/* Regression test for redmine issue #20701
*
* This test checks that the SHM transport will not listen on the same port
* in unicast and multicast at the same time.
* It does so by specifying custom default locators on a DataReader and then
* checking that the port mutation took place, thus producing a different port.
*/
TEST(SHM, SamePortUnicastMulticast)
{
PubSubReader<HelloWorldPubSubType> participant(TEST_TOPIC_NAME);

Locator locator;
locator.kind = LOCATOR_KIND_SHM;
locator.port = global_port;

LocatorList unicast_list;
LocatorList multicast_list;

// Note: this is using knowledge of the SHM locator address format since
// SHMLocator is not exposed to the user.
locator.address[0] = 'U';
unicast_list.push_back(locator);

// Note: this is using knowledge of the SHM locator address format since
// SHMLocator is not exposed to the user.
locator.address[0] = 'M';
multicast_list.push_back(locator);

// Create the reader with the custom transport and locators
auto testTransport = std::make_shared<SharedMemTransportDescriptor>();
participant
.disable_builtin_transport()
.add_user_transport_to_pparams(testTransport)
.set_default_unicast_locators(unicast_list)
.set_default_multicast_locators(multicast_list)
.init();

ASSERT_TRUE(participant.isInitialized());

// Retrieve the listening locators and check that one port is different
LocatorList reader_locators;
participant.get_native_reader().get_listening_locators(reader_locators);

ASSERT_EQ(reader_locators.size(), 2u);
auto it = reader_locators.begin();
auto first_port = it->port;
++it;
auto second_port = it->port;
EXPECT_NE(first_port, second_port);
EXPECT_TRUE(first_port == global_port || second_port == global_port);
}

TEST(SHM, Test300KFragmentation)
{
PubSubReader<Data1mbPubSubType> reader(TEST_TOPIC_NAME);
Expand Down
34 changes: 34 additions & 0 deletions test/unittest/transport/SharedMemTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,40 @@ TEST_F(SHMTransportTests, port_lock_read_exclusive)
port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive);
}

// Regression test for redmine issue #20701
TEST_F(SHMTransportTests, port_lock_read_shared_then_exclusive)
{
const std::string domain_name("SHMTests");

auto shared_mem_manager = SharedMemManager::create(domain_name);

shared_mem_manager->remove_port(0);

auto port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared);
ASSERT_THROW(shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive),
std::exception);

port.reset();
port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive);
}

// Regression test for redmine issue #20701
TEST_F(SHMTransportTests, port_lock_read_exclusive_then_shared)
{
const std::string domain_name("SHMTests");

auto shared_mem_manager = SharedMemManager::create(domain_name);

shared_mem_manager->remove_port(0);

auto port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadExclusive);
ASSERT_THROW(shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared),
std::exception);

port.reset();
port = shared_mem_manager->open_port(0, 1, 1000, SharedMemGlobal::Port::OpenMode::ReadShared);
}

TEST_F(SHMTransportTests, robust_exclusive_lock)
{
const std::string lock_name = "robust_exclusive_lock_test1_el";
Expand Down

0 comments on commit 54090bf

Please sign in to comment.