From 5c81b25fb26db28fc71d2fd63f7ddb285f2b55a3 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Wed, 15 Sep 2021 11:08:01 +0300 Subject: [PATCH] [#2088] fixed crash on server shutdown --- src/bin/agent/main.cc | 2 ++ src/bin/d2/d2_process.cc | 2 +- src/bin/d2/main.cc | 2 ++ src/bin/d2/tests/dns_client_unittests.cc | 19 +++++++++++-------- src/bin/netconf/main.cc | 2 ++ src/lib/process/d_controller.cc | 15 +++++++++++++++ src/lib/process/process_messages.cc | 2 ++ src/lib/process/process_messages.h | 1 + src/lib/process/process_messages.mes | 7 +++++++ 9 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/bin/agent/main.cc b/src/bin/agent/main.cc index 081993a8b3..68687083a6 100644 --- a/src/bin/agent/main.cc +++ b/src/bin/agent/main.cc @@ -43,5 +43,7 @@ int main(int argc, char* argv[]) { ret = EXIT_FAILURE; } + CtrlAgentController::instance().reset(); + return (ret); } diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index a4a738be24..c06e0bea9a 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -436,7 +436,7 @@ D2Process::reconfigureQueueMgr() { } D2Process::~D2Process() { -}; +} D2CfgMgrPtr D2Process::getD2CfgMgr() { diff --git a/src/bin/d2/main.cc b/src/bin/d2/main.cc index da7be90dc4..25975a77d3 100644 --- a/src/bin/d2/main.cc +++ b/src/bin/d2/main.cc @@ -52,5 +52,7 @@ int main(int argc, char* argv[]) { ret = EXIT_FAILURE; } + D2Controller::instance().reset(); + return (ret); } diff --git a/src/bin/d2/tests/dns_client_unittests.cc b/src/bin/d2/tests/dns_client_unittests.cc index 33369935ab..6d38cc3fac 100644 --- a/src/bin/d2/tests/dns_client_unittests.cc +++ b/src/bin/d2/tests/dns_client_unittests.cc @@ -5,20 +5,23 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include + #include #include #include #include #include #include +#include +#include + #include #include #include -#include -#include -#include #include +#include + using namespace std; using namespace isc; using namespace isc::asiolink; @@ -178,7 +181,7 @@ public: } // A response message is now ready to send. Send it! socket->send_to(boost::asio::buffer(response_buf.getData(), - response_buf.getLength()), + response_buf.getLength()), *remote); } @@ -212,7 +215,7 @@ public: TSIGContextPtr context; if (client_key) { - context.reset(new TSIGContext(*client_key)); + context = client_key->createContext(); } isc::util::InputBuffer received_data_buffer(receive_buffer_, @@ -287,7 +290,7 @@ public: // ensure that doUpdate doesn't throw an exception for valid timeouts. unsigned int timeout = DNSClient::getMaxTimeout(); EXPECT_NO_THROW(dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), - TEST_PORT, message, timeout)); + TEST_PORT, message, timeout)); // Cross the limit and expect that exception is thrown this time. timeout = DNSClient::getMaxTimeout() + 1; @@ -385,7 +388,7 @@ public: const int timeout = 500; expected_++; dns_client_->doUpdate(service_, IOAddress(TEST_ADDRESS), TEST_PORT, - message, timeout); + message, timeout); // It is possible to request that two packets are sent concurrently. if (two_sends) { @@ -410,7 +413,7 @@ public: // Performs a single request-response exchange with or without TSIG // // @param client_key TSIG passed to dns_client and also used by the - // ""server" to verify the request. + // "server" to verify the request. // request. // @param server_key TSIG key the "server" should use to sign the response. // If this is NULL, then client_key is used. diff --git a/src/bin/netconf/main.cc b/src/bin/netconf/main.cc index 4778b3bb8c..3481b5b171 100644 --- a/src/bin/netconf/main.cc +++ b/src/bin/netconf/main.cc @@ -43,5 +43,7 @@ int main(int argc, char* argv[]) { ret = EXIT_FAILURE; } + NetconfController::instance().reset(); + return (ret); } diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index 72bf80e027..4a2062cdb5 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -23,6 +24,7 @@ using namespace isc::asiolink; using namespace isc::data; using namespace isc::config; +using namespace isc::hooks; namespace ph = std::placeholders; namespace isc { @@ -805,6 +807,19 @@ DControllerBase::usage(const std::string & text) { } DControllerBase::~DControllerBase() { + // Explicitly unload hooks + HooksManager::prepareUnloadLibraries(); + if (!HooksManager::unloadLibraries()) { + auto names = HooksManager::getLibraryNames(); + std::string msg; + if (!names.empty()) { + msg = names[0]; + for (size_t i = 1; i < names.size(); ++i) { + msg += std::string(", ") + names[i]; + } + } + LOG_ERROR(dctl_logger, DCTL_UNLOAD_LIBRARIES_ERROR).arg(msg); + } } // Refer to config_report so it will be embedded in the binary diff --git a/src/lib/process/process_messages.cc b/src/lib/process/process_messages.cc index af290ed409..a4ee304d3d 100644 --- a/src/lib/process/process_messages.cc +++ b/src/lib/process/process_messages.cc @@ -35,6 +35,7 @@ extern const isc::log::MessageID DCTL_SHUTDOWN = "DCTL_SHUTDOWN"; extern const isc::log::MessageID DCTL_SHUTDOWN_SIGNAL_RECVD = "DCTL_SHUTDOWN_SIGNAL_RECVD"; extern const isc::log::MessageID DCTL_STANDALONE = "DCTL_STANDALONE"; extern const isc::log::MessageID DCTL_STARTING = "DCTL_STARTING"; +extern const isc::log::MessageID DCTL_UNLOAD_LIBRARIES_ERROR = "DCTL_UNLOAD_LIBRARIES_ERROR"; extern const isc::log::MessageID DCTL_UNSUPPORTED_SIGNAL = "DCTL_UNSUPPORTED_SIGNAL"; } // namespace process @@ -71,6 +72,7 @@ const char* values[] = { "DCTL_SHUTDOWN_SIGNAL_RECVD", "OS signal %1 received, starting shutdown", "DCTL_STANDALONE", "%1 skipping message queue, running standalone", "DCTL_STARTING", "%1 starting, pid: %2, version: %3 (%4)", + "DCTL_UNLOAD_LIBRARIES_ERROR", "error unloading hooks libraries during shutdown: %1", "DCTL_UNSUPPORTED_SIGNAL", "ignoring reception of unsupported signal: %1", NULL }; diff --git a/src/lib/process/process_messages.h b/src/lib/process/process_messages.h index 8e1334ea98..0ae950a520 100644 --- a/src/lib/process/process_messages.h +++ b/src/lib/process/process_messages.h @@ -36,6 +36,7 @@ extern const isc::log::MessageID DCTL_SHUTDOWN; extern const isc::log::MessageID DCTL_SHUTDOWN_SIGNAL_RECVD; extern const isc::log::MessageID DCTL_STANDALONE; extern const isc::log::MessageID DCTL_STARTING; +extern const isc::log::MessageID DCTL_UNLOAD_LIBRARIES_ERROR; extern const isc::log::MessageID DCTL_UNSUPPORTED_SIGNAL; } // namespace process diff --git a/src/lib/process/process_messages.mes b/src/lib/process/process_messages.mes index dc25ae3991..95d6cfcd82 100644 --- a/src/lib/process/process_messages.mes +++ b/src/lib/process/process_messages.mes @@ -87,6 +87,13 @@ to create and initialize its application instance. This error message is issued if the controller could not initialize the application and will exit. +% DCTL_UNLOAD_LIBRARIES_ERROR error unloading hooks libraries during shutdown: %1 +This error message indicates that during shutdown, unloading hooks +libraries failed to close them. If the list of libraries is empty it is +a programmatic error in the server code. If it is not empty it could be +a programmatic error in one of the hooks libraries which could lead to +a crash during finalization. + % DCTL_NOT_RUNNING %1 application instance is not running A warning message is issued when an attempt is made to shut down the application when it is not running.