Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix polling manager thread shut down #248

Merged
merged 1 commit into from
Aug 17, 2021
Merged
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
6 changes: 5 additions & 1 deletion email/src/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ Context::Context()

Context::~Context()
{
if (!is_valid()) {
logger_->debug("destroying");
if (is_valid()) {
(void)shutdown();
}
}
Expand All @@ -60,6 +61,7 @@ Context::init()
options_ = options.value();

is_valid_ = true;
logger_->debug("initialized");
}

void
Expand Down Expand Up @@ -97,9 +99,11 @@ Context::shutdown()
// Only call shutdown() if they have been init, otherwise they will get initialized
if (is_receiver_init_) {
get_receiver()->shutdown();
is_receiver_init_ = false;
}
if (is_polling_manager_init_) {
get_polling_manager()->shutdown();
is_polling_manager_init_ = false;
}
logger_->debug("shut down");
log::shutdown();
Expand Down
10 changes: 8 additions & 2 deletions email/src/email/polling_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ PollingManager::PollingManager(std::shared_ptr<EmailReceiver> receiver)
handlers_mutex_(),
handlers_(),
logger_(log::create("PollingManager"))
{}
{
logger_->debug("initialized");
}

PollingManager::~PollingManager()
{
logger_->debug("destroying");
shutdown();
}

void
Expand Down Expand Up @@ -76,8 +79,11 @@ PollingManager::shutdown()
receiver_->shutdown();
if (has_started()) {
do_shutdown_ = true;
thread_.join();
if (thread_.joinable()) {
thread_.join();
}
}
has_started_ = false;
logger_->debug("shut down");
}

Expand Down
2 changes: 2 additions & 0 deletions email/src/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ init(const Level & level)
root_logger = std::make_shared<spdlog::logger>("root", sinks.begin(), sinks.end());
root_logger->set_level(spdlog::level::debug);
spdlog::register_logger(root_logger);
root_logger->flush_on(spdlog::level::warn);
root_logger->debug("logging to file: {}", log_to_file ? "true" : "false");
root_logger->debug("logging level set to: {}", level_to_string(level));

Expand Down Expand Up @@ -205,6 +206,7 @@ shutdown()
root_logger->debug("shutting down");
spdlog::drop_all();
spdlog::shutdown();
root_logger->flush();
root_logger = nullptr;
sink_console = nullptr;
is_logging_initialized = false;
Expand Down
17 changes: 2 additions & 15 deletions rmw_email_cpp/include/rmw_email_cpp/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,10 @@ struct rmw_context_impl_t

bool is_shutdown{false};

rmw_context_impl_t()
{}
rmw_context_impl_t();

/// Context destruction, called when deleting this.
~rmw_context_impl_t()
{
// Shutdown middleware
email::shutdown();

// Make sure we didn't get destroyed while there's still a node
if (0u != this->node_count) {
RCUTILS_SAFE_FWRITE_TO_STDERR(
"Not all nodes were finished before finishing the context\n."
"Ensure `rmw_destroy_node` is called for all nodes before `rmw_context_fini`,"
"to avoid leaking.\n");
}
}
~rmw_context_impl_t();

/// Called whenever a new node is created.
rmw_ret_t
Expand Down
21 changes: 21 additions & 0 deletions rmw_email_cpp/src/rmw_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include "rmw_email_cpp/guard_condition.hpp"
#include "rmw_email_cpp/identifier.hpp"
#include "rmw_email_cpp/log.hpp"
#include "rmw_email_cpp/types.hpp"

extern "C" rmw_ret_t rmw_init_options_init(
Expand Down Expand Up @@ -102,6 +103,26 @@ extern "C" rmw_ret_t rmw_init_options_fini(rmw_init_options_t * init_options)
return ret;
}

rmw_context_impl_t::rmw_context_impl_t()
{
RMW_EMAIL_LOG_DEBUG("initializing context implementation");
}

rmw_context_impl_t::~rmw_context_impl_t()
{
RMW_EMAIL_LOG_DEBUG("shutting down context implementation");
// Shutdown middleware
email::shutdown();

// Make sure we didn't get destroyed while there's still a node
if (0u != this->node_count) {
RCUTILS_SAFE_FWRITE_TO_STDERR(
"Not all nodes were finished before finishing the context\n."
"Ensure `rmw_destroy_node` is called for all nodes before `rmw_context_fini`,"
"to avoid leaking.\n");
}
}

rmw_ret_t
rmw_context_impl_t::init(rmw_init_options_t * options, size_t domain_id)
{
Expand Down