Skip to content

Commit

Permalink
iox-#1036 Make 'MessageQueue' and 'UnixDomainSocket' non-nullable
Browse files Browse the repository at this point in the history
  • Loading branch information
elBoberido committed Sep 1, 2023
1 parent c6505cf commit 90e1a13
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ class MessageQueue
static constexpr uint64_t MAX_MESSAGE_SIZE = 4096;
static constexpr uint64_t MAX_MESSAGE_NUMBER = 10;

/// @todo iox-#1036 Remove when all channels are ported to the builder pattern
/// default constructor. The result is an invalid MessageQueue object which can be reassigned later by using the
/// move constructor.
MessageQueue() noexcept = default;

MessageQueue() noexcept = delete;
MessageQueue(const MessageQueue& other) = delete;
MessageQueue(MessageQueue&& other) noexcept;
MessageQueue& operator=(const MessageQueue& other) = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,7 @@ class UnixDomainSocket
using result_t = expected<UnixDomainSocket, IpcChannelError>;
using errorType_t = IpcChannelError;

/// @brief default constructor. The result is an invalid UnixDomainSocket object which can be reassigned later by
/// using the
/// move constructor.
UnixDomainSocket() noexcept = default;

UnixDomainSocket() noexcept = delete;
UnixDomainSocket(const UnixDomainSocket& other) = delete;
UnixDomainSocket(UnixDomainSocket&& other) noexcept;
UnixDomainSocket& operator=(const UnixDomainSocket& other) = delete;
Expand Down
17 changes: 6 additions & 11 deletions iceoryx_hoofs/test/moduletests/test_unix_domain_sockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ class UnixDomainSocket_test : public Test
public:
void SetUp() override
{
auto serverResult = UnixDomainSocket::create(
goodName, IpcChannelSide::SERVER, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber);
ASSERT_THAT(serverResult.has_error(), Eq(false));
server = std::move(serverResult.value());

auto clientResult = UnixDomainSocket::create(
goodName, IpcChannelSide::CLIENT, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber);
ASSERT_THAT(clientResult.has_error(), Eq(false));
client = std::move(clientResult.value());
}

void TearDown() override
Expand Down Expand Up @@ -118,8 +109,12 @@ class UnixDomainSocket_test : public Test
const std::chrono::milliseconds WAIT_IN_MS{10};
std::atomic_bool doWaitForThread{true};
static constexpr uint64_t MaxMsgNumber = 10U;
UnixDomainSocket server;
UnixDomainSocket client;
UnixDomainSocket server{
UnixDomainSocket::create(goodName, IpcChannelSide::SERVER, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber)
.expect("Valid UnixDomainSocket")};
UnixDomainSocket client{
UnixDomainSocket::create(goodName, IpcChannelSide::CLIENT, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber)
.expect("Valid UnixDomainSocket")};
};

constexpr uint64_t UnixDomainSocket_test::MaxMsgNumber;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "iceoryx_posh/internal/runtime/ipc_message.hpp"
#include "iox/deadline_timer.hpp"
#include "iox/duration.hpp"
#include "iox/optional.hpp"
#include "iox/relative_pointer.hpp"

#include "iceoryx_dust/posix_wrapper/message_queue.hpp"
Expand Down Expand Up @@ -265,7 +266,7 @@ class IpcInterface
uint64_t m_maxMessageSize{0U};
uint64_t m_maxMessages{0U};
iox::posix::IpcChannelSide m_channelSide{posix::IpcChannelSide::CLIENT};
IpcChannelType m_ipcChannel;
optional<IpcChannelType> m_ipcChannel;
};

using IpcInterfaceBase = IpcInterface<platform::IoxIpcChannelType>;
Expand Down
44 changes: 35 additions & 9 deletions iceoryx_posh/source/runtime/ipc_interface_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ IpcInterface<IpcChannelType>::IpcInterface(const RuntimeName_t& runtimeName,
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::receive(IpcMessage& answer) const noexcept
{
auto message = m_ipcChannel.receive();
if (!m_ipcChannel.has_value())
{
IOX_LOG(WARN) << "Trying to receive data on an non-initialized IPC interface! Interface name: "
<< m_runtimeName;
return false;
}

auto message = m_ipcChannel->receive();
if (message.has_error())
{
return false;
Expand All @@ -90,7 +97,14 @@ bool IpcInterface<IpcChannelType>::receive(IpcMessage& answer) const noexcept
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::timedReceive(const units::Duration timeout, IpcMessage& answer) const noexcept
{
return !m_ipcChannel.timedReceive(timeout)
if (!m_ipcChannel.has_value())
{
IOX_LOG(WARN) << "Trying to receive data on an non-initialized IPC interface! Interface name: "
<< m_runtimeName;
return false;
}

return !m_ipcChannel->timedReceive(timeout)
.and_then([&answer](auto& message) {
IpcInterface<IpcChannelType>::setMessageFromString(message.c_str(), answer);
})
Expand All @@ -113,6 +127,12 @@ bool IpcInterface<IpcChannelType>::setMessageFromString(const char* buffer, IpcM
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::send(const IpcMessage& msg) const noexcept
{
if (!m_ipcChannel.has_value())
{
IOX_LOG(WARN) << "Trying to send data on an non-initialized IPC interface! Interface name: " << m_runtimeName;
return false;
}

if (!msg.isValid())
{
IOX_LOG(ERROR) << "Trying to send the message " << msg.getMessage() << " which "
Expand All @@ -127,12 +147,18 @@ bool IpcInterface<IpcChannelType>::send(const IpcMessage& msg) const noexcept
IOX_LOG(ERROR) << "msg size of " << messageSize << " bigger than configured max message size";
}
};
return !m_ipcChannel.send(msg.getMessage()).or_else(logLengthError).has_error();
return !m_ipcChannel->send(msg.getMessage()).or_else(logLengthError).has_error();
}

template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::timedSend(const IpcMessage& msg, units::Duration timeout) const noexcept
{
if (!m_ipcChannel.has_value())
{
IOX_LOG(WARN) << "Trying to send data on an non-initialized IPC interface! Interface name: " << m_runtimeName;
return false;
}

if (!msg.isValid())
{
IOX_LOG(ERROR) << "Trying to send the message " << msg.getMessage() << " which "
Expand All @@ -147,7 +173,7 @@ bool IpcInterface<IpcChannelType>::timedSend(const IpcMessage& msg, units::Durat
IOX_LOG(ERROR) << "msg size of " << messageSize << " bigger than configured max message size";
}
};
return !m_ipcChannel.timedSend(msg.getMessage(), timeout).or_else(logLengthError).has_error();
return !m_ipcChannel->timedSend(msg.getMessage(), timeout).or_else(logLengthError).has_error();
}

template <typename IpcChannelType>
Expand All @@ -159,20 +185,20 @@ const RuntimeName_t& IpcInterface<IpcChannelType>::getRuntimeName() const noexce
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::isInitialized() const noexcept
{
return m_ipcChannel.isInitialized();
return m_ipcChannel.has_value();
}

template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::openIpcChannel(const posix::IpcChannelSide channelSide) noexcept
{
m_channelSide = channelSide;
IpcChannelType::create(m_runtimeName, m_channelSide, m_maxMessageSize, m_maxMessages)
.and_then([this](auto& ipcChannel) { this->m_ipcChannel = std::move(ipcChannel); })
.and_then([this](auto& ipcChannel) { this->m_ipcChannel.emplace(std::move(ipcChannel)); })
.or_else([](auto& err) {
IOX_LOG(ERROR) << "unable to create ipc channel with error code: " << static_cast<uint8_t>(err);
});

return m_ipcChannel.isInitialized();
return isInitialized();
}

template <typename IpcChannelType>
Expand All @@ -184,7 +210,7 @@ bool IpcInterface<IpcChannelType>::reopen() noexcept
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::ipcChannelMapsToFile() noexcept
{
return !m_ipcChannel.isOutdated().value_or(true);
return m_ipcChannel.has_value() && !m_ipcChannel->isOutdated().value_or(true);
}

template <>
Expand All @@ -202,7 +228,7 @@ bool IpcInterface<posix::NamedPipe>::ipcChannelMapsToFile() noexcept
template <typename IpcChannelType>
bool IpcInterface<IpcChannelType>::hasClosableIpcChannel() const noexcept
{
return m_ipcChannel.isInitialized();
return isInitialized();
}

template <typename IpcChannelType>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,9 @@ class StringToMessage : public IpcInterfaceBase
class CMqInterfaceStartupRace_test : public Test
{
public:
CMqInterfaceStartupRace_test()
: m_appQueue{platform::IoxIpcChannelType::create()}
{
}

virtual void SetUp()
{
ASSERT_THAT(m_roudiQueue.has_error(), false);
ASSERT_THAT(m_roudiQueue.has_value(), true);
}
virtual void TearDown()
{
Expand Down Expand Up @@ -102,11 +97,13 @@ class CMqInterfaceStartupRace_test : public Test
regAck << IpcMessageTypeToString(IpcMessageType::REG_ACK) << DUMMY_SHM_SIZE << DUMMY_SHM_OFFSET
<< oldMsg.getElementAtIndex(INDEX_OF_TIMESTAMP) << DUMMY_SEGMENT_ID << SEND_KEEP_ALIVE;

if (m_appQueue.has_error())
if (!m_appQueue.has_value())
{
m_appQueue = platform::IoxIpcChannelType::create(MqAppName, IpcChannelSide::CLIENT);
platform::IoxIpcChannelType::create(MqAppName, IpcChannelSide::CLIENT).and_then([this](auto& channel) {
this->m_appQueue.emplace(std::move(channel));
});
}
ASSERT_THAT(m_appQueue.has_error(), false);
ASSERT_THAT(m_appQueue.has_value(), true);

ASSERT_FALSE(m_appQueue->send(regAck.getMessage()).has_error());
}
Expand All @@ -116,7 +113,7 @@ class CMqInterfaceStartupRace_test : public Test
platform::IoxIpcChannelType::result_t m_roudiQueue{
platform::IoxIpcChannelType::create(roudi::IPC_CHANNEL_ROUDI_NAME, IpcChannelSide::SERVER)};
std::mutex m_appQueueMutex;
platform::IoxIpcChannelType::result_t m_appQueue;
optional<platform::IoxIpcChannelType> m_appQueue;
};

#if !defined(__APPLE__)
Expand Down

0 comments on commit 90e1a13

Please sign in to comment.