Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ bool ControlClientChannel::getResponse(ControlClientMessage& msg)
LM_LOG_DEBUG() << "Response retrieved.";
}

nudgeControlClientHandler();

return result;
}

Expand All @@ -89,25 +91,7 @@ void ControlClientChannel::sendRequest(ControlClientMessage& msg)
request_.msg_ = msg;
request_.empty_ = false;

// now map the semaphore and post on it
// Attempt to map the semaphore
auto* nudgeLM = mmap(
NULL, sizeof(osal::Semaphore), PROT_WRITE, MAP_SHARED, osal::IpcCommsSync::control_client_handler_nudge_fd, 0);

// RULECHECKER_comment(1, 1, check_c_style_cast, "This is the definition provided by the OS and does a C-style
// cast.", true)
if (nudgeLM != MAP_FAILED)
{
LM_LOG_DEBUG() << "Request sent. Waiting for acknowledgment...";
auto* semaphore = static_cast<osal::Semaphore*>(nudgeLM);

// coverity[cert_mem52_cpp_violation:FALSE] The allocated memory is checked by the containing if statement.
const auto result = semaphore->post();
SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(result == osal::OsalReturnType::kSuccess,
"ControlClientChannel semaphore post failed");

munmap(nudgeLM, sizeof(osal::Semaphore)); // Unmap the semaphore
}
nudgeControlClientHandler();

const auto result = nudge_LM_Handler_.wait();
SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE(result == osal::OsalReturnType::kSuccess,
Expand Down Expand Up @@ -196,6 +180,9 @@ ControlClientChannelP ControlClientChannel::initializeControlClientChannel(int f
lock.unlock();
init_cv_.notify_all();
}

loadControlNudge();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check for errors here?


return result;
}

Expand Down Expand Up @@ -260,6 +247,29 @@ void ControlClientChannel::releaseParentMapping()
ipc_parent_.reset();
}

bool ControlClientChannel::loadControlNudge() {
if (nudgeControlClientHandler_ != nullptr) {
LM_LOG_ERROR() << "Semaphore was already mapped!";
return false;
}

void* nudgeBuf = mmap(
NULL, sizeof(osal::Semaphore), PROT_WRITE, MAP_SHARED,
osal::IpcCommsSync::control_client_handler_nudge_fd, 0);

// RULECHECKER_comment(1, 1, check_c_style_cast, "This is the definition provided by the OS and does a C-style cast.", true)
if (nudgeBuf == MAP_FAILED)
{
LM_LOG_ERROR() << "mmap of nudge semaphore failed in initializeControlClientChannel:"
<< std::strerror(errno);
return false;
}

nudgeControlClientHandler_ = static_cast<osal::Semaphore*>(nudgeBuf);
return true;
}


std::string_view ControlClientChannel::toString(ControlClientCode code)
{
for (const auto& mapping : stateArray)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ class ControlClientChannel final {

private:

/// @brief loads the control channel's nudge semaphore.
static bool loadControlNudge();

/// @brief Ensure that the ControlClientChannel was setup properly before
/// accessing it
static bool is_initialized_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ static std::atomic_bool em_cancelled{false};
static void my_signal_handler(int)
{
em_cancelled.store(true);
ControlClientChannel::nudgeControlClientHandler();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: nudgeControlClientHandler uses SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE and we do not have a signal-safe assertion handler right now.
It also uses LM_LOG_DEBUG to log a message, which probably also not valid to call from a signal handler.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be called before the control client is setup? E.g. launch manager is started but directly receives a SIGTERM/SIGINT

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once #303 is merged we can use the new signal-safe logging functions from there.

}

void ProcessGroupManager::cancel()
Expand Down Expand Up @@ -142,16 +143,13 @@ inline bool ProcessGroupManager::initializeControlClientHandler()
// The name is removed from the file system after creation, memory
// is mapped and a pointer stored, the FD is kept open.
ControlClientChannel::nudgeControlClientHandler_ = nullptr;
char shm_name[static_cast<uint32_t>(score::lcm::internal::ProcessLimits::maxLocalBuffSize)];
constexpr static std::string_view shm_name{"/_nudge~._.~me_"};

static_cast<void>(snprintf(shm_name,
static_cast<uint32_t>(score::lcm::internal::ProcessLimits::maxLocalBuffSize),
"/_nudge~._.~me_")); // random name
int fd = shm_open(shm_name, O_CREAT | O_EXCL | O_RDWR, 0U);
int fd = shm_open(shm_name.data(), O_CREAT | O_EXCL | O_RDWR, 0U);

if (fd >= 0)
{
shm_unlink(shm_name);
shm_unlink(shm_name.data());

if (0 == ftruncate(fd, static_cast<off_t>(sizeof(osal::Semaphore))))
{
Expand Down Expand Up @@ -464,10 +462,6 @@ bool ProcessGroupManager::sendResponse(ControlClientMessage msg)
<< static_cast<int>(msg.request_or_response_) << ") re state"
<< msg.process_group_state_.pg_state_name_ << "of PG" << msg.process_group_state_.pg_name_;
ret = scc->sendResponse(msg);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to clean up this variable which is not necessary since the if was removed

Suggested change
ret = scc->sendResponse(msg);
return scc->sendResponse(msg);

if (!ret)
{
ControlClientChannel::nudgeControlClientHandler();
}
}
}

Expand Down
Loading