diff options
| author | Razvan Becheriu <razvan@isc.org> | 2021-02-16 09:03:21 +0100 |
|---|---|---|
| committer | Razvan Becheriu <razvan@isc.org> | 2021-02-18 18:23:57 +0100 |
| commit | 9adc0b96bfa4a1e23ff67b8990eeab0f9d862885 (patch) | |
| tree | 66785fbc4aa159ffd7530cd1dd203dc55f240a47 | |
| parent | [#899] fixed CI (diff) | |
| download | kea-9adc0b96bfa4a1e23ff67b8990eeab0f9d862885.tar.xz kea-9adc0b96bfa4a1e23ff67b8990eeab0f9d862885.zip | |
[#899] use IOSignalSet to handle signals
41 files changed, 209 insertions, 1221 deletions
diff --git a/src/bin/agent/tests/ca_controller_unittests.cc b/src/bin/agent/tests/ca_controller_unittests.cc index 2be67dfe46..2018ac28ce 100644 --- a/src/bin/agent/tests/ca_controller_unittests.cc +++ b/src/bin/agent/tests/ca_controller_unittests.cc @@ -5,6 +5,8 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include <config.h> + +#include <asiolink/testutils/timed_signal.h> #include <agent/ca_controller.h> #include <agent/ca_process.h> #include <agent/ca_command_mgr.h> @@ -15,12 +17,13 @@ #include <sstream> #include <unistd.h> -using namespace std; +using namespace isc::asiolink::test; using namespace isc::agent; using namespace isc::data; using namespace isc::http; using namespace isc::process; using namespace boost::posix_time; +using namespace std; namespace { diff --git a/src/bin/d2/tests/d2_controller_unittests.cc b/src/bin/d2/tests/d2_controller_unittests.cc index 458c2a66d6..cc52354bf9 100644 --- a/src/bin/d2/tests/d2_controller_unittests.cc +++ b/src/bin/d2/tests/d2_controller_unittests.cc @@ -6,6 +6,7 @@ #include <config.h> +#include <asiolink/testutils/timed_signal.h> #include <cc/command_interpreter.h> #include <d2/d2_controller.h> #include <d2/d2_process.h> @@ -18,6 +19,7 @@ #include <sstream> +using namespace isc::asiolink::test; using namespace isc::process; using namespace boost::posix_time; diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 03f4765385..1f99921633 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -111,9 +111,11 @@ ControlledDhcpv4Srv::init(const std::string& file_name) { // Set signal handlers. When the SIGHUP is received by the process // the server reconfiguration will be triggered. When SIGTERM or // SIGINT will be received, the server will start shutting down. - signal_set_.reset(new isc::util::SignalSet(SIGINT, SIGHUP, SIGTERM)); - // Set the pointer to the handler function. - signal_handler_ = signalHandler; + signal_set_.reset(new IOSignalSet(getIOService(), signalHandler)); + + signal_set_->add(SIGINT); + signal_set_->add(SIGHUP); + signal_set_->add(SIGTERM); } void ControlledDhcpv4Srv::cleanup() { diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index ff7bdb54e7..15ad3325fd 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -990,37 +990,11 @@ Dhcpv4Srv::run_one() { // second. } catch (const SignalInterruptOnSelect&) { - // Packet reception interrupted because a signal has been received. - // This is not an error because we might have received a SIGTERM, - // SIGINT, SIGHUP or SIGCHLD which are handled by the server. For - // signals that are not handled by the server we rely on the default - // behavior of the system. - LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL, DHCP4_BUFFER_WAIT_SIGNAL) - .arg(signal_set_->getNext()); } catch (const std::exception& e) { // Log all other errors. LOG_ERROR(packet4_logger, DHCP4_BUFFER_RECEIVE_FAIL).arg(e.what()); } - // Handle next signal received by the process. It must be called after - // an attempt to receive a packet to properly handle server shut down. - // The SIGTERM or SIGINT will be received prior to, or during execution - // of select() (select is invoked by receivePacket()). When that - // happens, select will be interrupted. The signal handler will be - // invoked immediately after select(). The handler will set the - // shutdown flag and cause the process to terminate before the next - // select() function is called. If the function was called before - // receivePacket the process could wait up to the duration of timeout - // of select() to terminate. - try { - handleSignal(); - } catch (const std::exception& e) { - // Standard exception occurred. Let's be on the safe side to - // catch std::exception. - LOG_ERROR(dhcp4_logger, DHCP4_HANDLE_SIGNAL_EXCEPTION) - .arg(e.what()); - } - // Timeout may be reached or signal received, which breaks select() // with no reception occurred. No need to log anything here because // we have logged right after the call to receivePacket(). diff --git a/src/bin/dhcp4/tests/kea_controller_unittest.cc b/src/bin/dhcp4/tests/kea_controller_unittest.cc index 1f4d0091e7..4fec95314b 100644 --- a/src/bin/dhcp4/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp4/tests/kea_controller_unittest.cc @@ -186,8 +186,6 @@ public: // stub implementation used in tests. cb_control_.reset(new TestCBControlDHCPv4()); } - - using ControlledDhcpv4Srv::signal_handler_; }; /// @brief test class for Kea configuration backend @@ -1060,7 +1058,7 @@ testBackendReconfiguration(const std::string& backend_first, // Explicitly calling signal handler for SIGHUP to trigger server // reconfiguration. - srv->signal_handler_(SIGHUP); + raise(SIGHUP); // The backend should have been created and its type should be // correct. diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index 7fed6c5c57..1afbbc63e3 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -186,9 +186,11 @@ ControlledDhcpv6Srv::init(const std::string& file_name) { // Set signal handlers. When the SIGHUP is received by the process // the server reconfiguration will be triggered. When SIGTERM or // SIGINT will be received, the server will start shutting down. - signal_set_.reset(new isc::util::SignalSet(SIGINT, SIGHUP, SIGTERM)); - // Set the pointer to the handler function. - signal_handler_ = signalHandler; + signal_set_.reset(new IOSignalSet(getIOService(), signalHandler)); + + signal_set_->add(SIGINT); + signal_set_->add(SIGHUP); + signal_set_->add(SIGTERM); } void ControlledDhcpv6Srv::cleanup() { diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index d18e316b83..e0f984349b 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -574,35 +574,10 @@ void Dhcpv6Srv::run_one() { // second. } catch (const SignalInterruptOnSelect&) { - // Packet reception interrupted because a signal has been received. - // This is not an error because we might have received a SIGTERM, - // SIGINT or SIGHUP which are handled by the server. For signals - // that are not handled by the server we rely on the default - // behavior of the system. - LOG_DEBUG(packet6_logger, DBG_DHCP6_DETAIL, DHCP6_BUFFER_WAIT_SIGNAL) - .arg(signal_set_->getNext()); } catch (const std::exception& e) { LOG_ERROR(packet6_logger, DHCP6_PACKET_RECEIVE_FAIL).arg(e.what()); } - // Handle next signal received by the process. It must be called after - // an attempt to receive a packet to properly handle server shut down. - // The SIGTERM or SIGINT will be received prior to, or during execution - // of select() (select is invoked by receivePacket()). When that happens, - // select will be interrupted. The signal handler will be invoked - // immediately after select(). The handler will set the shutdown flag - // and cause the process to terminate before the next select() function - // is called. If the function was called before receivePacket the - // process could wait up to the duration of timeout of select() to - // terminate. - try { - handleSignal(); - } catch (const std::exception& e) { - // An (a standard or ISC) exception occurred. - LOG_ERROR(dhcp6_logger, DHCP6_HANDLE_SIGNAL_EXCEPTION) - .arg(e.what()); - } - // Timeout may be reached or signal received, which breaks select() // with no packet received if (!query) { diff --git a/src/bin/dhcp6/tests/kea_controller_unittest.cc b/src/bin/dhcp6/tests/kea_controller_unittest.cc index d74924839b..a7e3c4de61 100644 --- a/src/bin/dhcp6/tests/kea_controller_unittest.cc +++ b/src/bin/dhcp6/tests/kea_controller_unittest.cc @@ -183,8 +183,6 @@ public: // stub implementation used in tests. cb_control_.reset(new TestCBControlDHCPv6()); } - - using ControlledDhcpv6Srv::signal_handler_; }; @@ -1046,7 +1044,7 @@ testBackendReconfiguration(const std::string& backend_first, // Explicitly calling signal handler for SIGHUP to trigger server // reconfiguration. - srv->signal_handler_(SIGHUP); + raise(SIGHUP); // The backend should have been created and its type should be // correct. diff --git a/src/bin/netconf/netconf_controller.cc b/src/bin/netconf/netconf_controller.cc index bdf2102c09..353a57fa59 100644 --- a/src/bin/netconf/netconf_controller.cc +++ b/src/bin/netconf/netconf_controller.cc @@ -10,6 +10,7 @@ #include <netconf/netconf_controller.h> #include <netconf/netconf_process.h> #include <netconf/parser_context.h> +#include <signal.h> using namespace isc::process; diff --git a/src/bin/netconf/tests/netconf_controller_unittests.cc b/src/bin/netconf/tests/netconf_controller_unittests.cc index 1b26a9f198..9d9806a738 100644 --- a/src/bin/netconf/tests/netconf_controller_unittests.cc +++ b/src/bin/netconf/tests/netconf_controller_unittests.cc @@ -5,6 +5,8 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include <config.h> + +#include <asiolink/testutils/timed_signal.h> #include <netconf/netconf_controller.h> #include <netconf/netconf_process.h> #include <cc/data.h> @@ -12,12 +14,13 @@ #include <boost/pointer_cast.hpp> #include <sstream> -using namespace std; +using namespace isc::asiolink::test; using namespace isc::netconf; using namespace isc::data; using namespace isc::http; using namespace isc::process; using namespace boost::posix_time; +using namespace std; namespace { diff --git a/src/hooks/dhcp/run_script/run_script.cc b/src/hooks/dhcp/run_script/run_script.cc index bc0a06a87c..91b43442ed 100644 --- a/src/hooks/dhcp/run_script/run_script.cc +++ b/src/hooks/dhcp/run_script/run_script.cc @@ -8,6 +8,7 @@ #include <run_script.h> +using namespace isc::asiolink; using namespace isc::data; using namespace isc::hooks; using namespace isc::util; @@ -17,6 +18,8 @@ using namespace std; namespace isc { namespace run_script { +IOServicePtr RunScriptImpl::io_service_; + RunScriptImpl::RunScriptImpl() : name_(), sync_(false) { } @@ -44,7 +47,7 @@ RunScriptImpl::configure(LibraryHandle& handle) { void RunScriptImpl::runScript(const ProcessArgs& args, const ProcessEnvVars& vars) { - ProcessSpawn process(name_, args, vars); + ProcessSpawn process(io_service_, name_, args, vars); process.spawn(); } diff --git a/src/hooks/dhcp/run_script/run_script.h b/src/hooks/dhcp/run_script/run_script.h index 692b43fad0..2e68240f4c 100644 --- a/src/hooks/dhcp/run_script/run_script.h +++ b/src/hooks/dhcp/run_script/run_script.h @@ -7,6 +7,7 @@ #ifndef RUN_SCRIPT_H #define RUN_SCRIPT_H +#include <asiolink/process_spawn.h> #include <dhcp/duid.h> #include <dhcp/hwaddr.h> #include <dhcp/option6_ia.h> @@ -15,7 +16,6 @@ #include <dhcpsrv/lease.h> #include <dhcpsrv/subnet.h> #include <hooks/library_handle.h> -#include <util/process_spawn.h> #include <string> namespace isc { @@ -30,12 +30,19 @@ public: /// @brief Destructor. ~RunScriptImpl(); + /// @brief Sets IO service to be used by the @ref ProcessSpawn instance. + /// + /// @param io_service The IOService object, used for all ASIO operations. + static void setIOService(const isc::asiolink::IOServicePtr& io_service) { + io_service_ = io_service; + } + /// @brief Extract boolean data and append to environment. /// /// @param value The value to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractBoolean(isc::util::ProcessEnvVars& vars, + static void extractBoolean(isc::asiolink::ProcessEnvVars& vars, const bool value, const std::string& prefix = "", const std::string& sufix = ""); @@ -45,7 +52,7 @@ public: /// @param value The value to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractInteger(isc::util::ProcessEnvVars& vars, + static void extractInteger(isc::asiolink::ProcessEnvVars& vars, const uint64_t value, const std::string& prefix = "", const std::string& sufix = ""); @@ -55,7 +62,7 @@ public: /// @param value The value to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractString(isc::util::ProcessEnvVars& vars, + static void extractString(isc::asiolink::ProcessEnvVars& vars, const std::string& value, const std::string& prefix = "", const std::string& sufix = ""); @@ -65,7 +72,7 @@ public: /// @param value The hwaddr to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractHWAddr(isc::util::ProcessEnvVars& vars, + static void extractHWAddr(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::HWAddrPtr& hwaddr, const std::string& prefix = "", const std::string& sufix = ""); @@ -75,7 +82,7 @@ public: /// @param value The duid to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractDUID(isc::util::ProcessEnvVars& vars, + static void extractDUID(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::DuidPtr duid, const std::string& prefix = "", const std::string& sufix = ""); @@ -85,7 +92,7 @@ public: /// @param value The option6IA to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractOptionIA(isc::util::ProcessEnvVars& vars, + static void extractOptionIA(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Option6IAPtr option6IA, const std::string& prefix = "", const std::string& sufix = ""); @@ -95,7 +102,7 @@ public: /// @param value The subnet4 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractSubnet4(isc::util::ProcessEnvVars& vars, + static void extractSubnet4(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Subnet4Ptr subnet4, const std::string& prefix = "", const std::string& sufix = ""); @@ -105,7 +112,7 @@ public: /// @param value The subnet6 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractSubnet6(isc::util::ProcessEnvVars& vars, + static void extractSubnet6(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Subnet6Ptr subnet6, const std::string& prefix = "", const std::string& sufix = ""); @@ -115,7 +122,7 @@ public: /// @param value The lease4 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractLease4(isc::util::ProcessEnvVars& vars, + static void extractLease4(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Lease4Ptr& lease4, const std::string& prefix = "", const std::string& sufix = ""); @@ -125,7 +132,7 @@ public: /// @param value The lease6 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractLease6(isc::util::ProcessEnvVars& vars, + static void extractLease6(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Lease6Ptr& lease6, const std::string& prefix = "", const std::string& sufix = ""); @@ -135,7 +142,7 @@ public: /// @param value The leases4 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractLeases4(isc::util::ProcessEnvVars& vars, + static void extractLeases4(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Lease4CollectionPtr& leases4, const std::string& prefix = "", const std::string& sufix = ""); @@ -145,7 +152,7 @@ public: /// @param value The leases6 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractLeases6(isc::util::ProcessEnvVars& vars, + static void extractLeases6(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Lease6CollectionPtr& leases6, const std::string& prefix = "", const std::string& sufix = ""); @@ -155,7 +162,7 @@ public: /// @param value The pkt4 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractPkt4(isc::util::ProcessEnvVars& vars, + static void extractPkt4(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Pkt4Ptr& pkt4, const std::string& prefix = "", const std::string& sufix = ""); @@ -165,7 +172,7 @@ public: /// @param value The pkt6 to be exported to target script environment. /// @param prefix The prefix for the name of the environment variable. /// @param sufix The sufix for the name of the environment variable. - static void extractPkt6(isc::util::ProcessEnvVars& vars, + static void extractPkt6(isc::asiolink::ProcessEnvVars& vars, const isc::dhcp::Pkt6Ptr& pkt6, const std::string& prefix = "", const std::string& sufix = ""); @@ -175,8 +182,8 @@ public: /// @param args The arguments for the target script. /// @param vars The environment variables made available for the target /// script. - void runScript(const isc::util::ProcessArgs& args, - const isc::util::ProcessEnvVars& vars); + void runScript(const isc::asiolink::ProcessArgs& args, + const isc::asiolink::ProcessEnvVars& vars); /// @brief Set name of the target script. /// @@ -219,6 +226,9 @@ private: /// exits, otherwise the call will return immediately after the script is /// started. bool sync_; + + /// @brief The IOService object, used for all ASIO operations. + static isc::asiolink::IOServicePtr io_service_; }; /// @brief The type of shared pointers to Run Script implementations. diff --git a/src/hooks/dhcp/run_script/run_script_callouts.cc b/src/hooks/dhcp/run_script/run_script_callouts.cc index c16c24b4a9..50ca846f8c 100644 --- a/src/hooks/dhcp/run_script/run_script_callouts.cc +++ b/src/hooks/dhcp/run_script/run_script_callouts.cc @@ -25,6 +25,7 @@ RunScriptImplPtr impl; } // namespace isc using namespace isc; +using namespace isc::asiolink; using namespace isc::data; using namespace isc::dhcp; using namespace isc::hooks; @@ -63,6 +64,40 @@ int unload() { return (0); } +/// @brief dhcp4_srv_configured callout implementation. +/// +/// @param handle callout handle. +int dhcp4_srv_configured(CalloutHandle& handle) { + try { + isc::asiolink::IOServicePtr io_service; + handle.getArgument("io_context", io_service); + RunScriptImpl::setIOService(io_service); + + } catch (const std::exception& ex) { + LOG_ERROR(run_script_logger, RUN_SCRIPT_LOAD_ERROR) + .arg(ex.what()); + return (1); + } + return (0); +} + +/// @brief dhcp6_srv_configured callout implementation. +/// +/// @param handle callout handle. +int dhcp6_srv_configured(CalloutHandle& handle) { + try { + isc::asiolink::IOServicePtr io_service; + handle.getArgument("io_context", io_service); + RunScriptImpl::setIOService(io_service); + + } catch (const std::exception& ex) { + LOG_ERROR(run_script_logger, RUN_SCRIPT_LOAD_ERROR) + .arg(ex.what()); + return (1); + } + return (0); +} + /// @brief handle @ref lease4_renew hook and set environment parameters for the /// script. /// IN: query4 subnet4 clientid hwaddr lease4 diff --git a/src/lib/asiolink/Makefile.am b/src/lib/asiolink/Makefile.am index b3b1a2ff17..feb442baba 100644 --- a/src/lib/asiolink/Makefile.am +++ b/src/lib/asiolink/Makefile.am @@ -24,7 +24,9 @@ libkea_asiolink_la_SOURCES += io_asio_socket.h libkea_asiolink_la_SOURCES += io_endpoint.cc io_endpoint.h libkea_asiolink_la_SOURCES += io_error.h libkea_asiolink_la_SOURCES += io_service.h io_service.cc +libkea_asiolink_la_SOURCES += io_service_signal.cc io_service_signal.h libkea_asiolink_la_SOURCES += io_socket.h io_socket.cc +libkea_asiolink_la_SOURCES += process_spawn.h process_spawn.cc libkea_asiolink_la_SOURCES += tcp_acceptor.h libkea_asiolink_la_SOURCES += tcp_endpoint.h libkea_asiolink_la_SOURCES += tcp_socket.h @@ -55,7 +57,9 @@ libkea_asiolink_include_HEADERS = \ io_endpoint.h \ io_error.h \ io_service.h \ + io_service_signal.h \ io_socket.h \ + process_spawn.h \ tcp_acceptor.h \ tcp_endpoint.h \ tcp_socket.h \ diff --git a/src/lib/process/io_service_signal.cc b/src/lib/asiolink/io_service_signal.cc index 0e80723f89..44c7658a14 100644 --- a/src/lib/process/io_service_signal.cc +++ b/src/lib/asiolink/io_service_signal.cc @@ -6,8 +6,7 @@ #include <config.h> -#include <process/d_log.h> -#include <process/io_service_signal.h> +#include <asiolink/io_service_signal.h> #include <exceptions/exceptions.h> #include <boost/enable_shared_from_this.hpp> @@ -15,17 +14,15 @@ #include <boost/asio/signal_set.hpp> #include <functional> -using namespace isc::asiolink; namespace ph = std::placeholders; namespace isc { -namespace process { +namespace asiolink { /// @brief Implementation class of IOSignalSet. class IOSignalSetImpl : public boost::enable_shared_from_this<IOSignalSetImpl>, - public boost::noncopyable -{ + public boost::noncopyable { public: /// @brief Constructor. /// @@ -73,10 +70,6 @@ IOSignalSetImpl::callback(const boost::system::error_code& ec, int signum) { try { handler_(signum); } catch (const std::exception& ex) { - // We log it and swallow it so we don't undermine IOService::run. - LOG_ERROR(dctl_logger, DCTL_SIGNAL_ERROR) - .arg(signum) - .arg(ex.what()); } } } @@ -98,8 +91,7 @@ IOSignalSetImpl::add(int signum) { } IOSignalSet::IOSignalSet(IOServicePtr io_service, IOSignalHandler handler) : - impl_(new IOSignalSetImpl(io_service, handler)) -{ + impl_(new IOSignalSetImpl(io_service, handler)) { // It can throw but the error is fatal... impl_->install(); } @@ -113,5 +105,5 @@ IOSignalSet::add(int signum) { impl_->add(signum); } -} // end of isc::process namespace -} // end of isc namespace +} // namespace asiolink +} // namespace isc diff --git a/src/lib/process/io_service_signal.h b/src/lib/asiolink/io_service_signal.h index aa0691f054..7efabf9646 100644 --- a/src/lib/process/io_service_signal.h +++ b/src/lib/asiolink/io_service_signal.h @@ -12,7 +12,7 @@ #include <boost/shared_ptr.hpp> namespace isc { -namespace process { +namespace asiolink { /// @brief Defines a handler function for an IOSignal. typedef std::function<void(int signum)> IOSignalHandler; @@ -48,7 +48,7 @@ private: /// @brief Defines a pointer to an IOSignalSet. typedef boost::shared_ptr<IOSignalSet> IOSignalSetPtr; -}; // end of isc::process namespace -}; // end of isc namespace +} // namespace asiolink +} // namespace isc #endif // IO_SERVICE_SIGNAL_H diff --git a/src/lib/util/process_spawn.cc b/src/lib/asiolink/process_spawn.cc index ad24597f14..55047439ac 100644 --- a/src/lib/util/process_spawn.cc +++ b/src/lib/asiolink/process_spawn.cc @@ -6,22 +6,24 @@ #include <config.h> +#include <asiolink/io_service_signal.h> +#include <asiolink/process_spawn.h> #include <exceptions/exceptions.h> -#include <util/process_spawn.h> -#include <util/signal_set.h> #include <cstring> #include <functional> #include <map> +#include <mutex> #include <signal.h> #include <stdlib.h> #include <errno.h> #include <unistd.h> #include <sys/wait.h> +using namespace std; namespace ph = std::placeholders; namespace isc { -namespace util { +namespace asiolink { /// @brief Type for process state struct ProcessState { @@ -37,11 +39,13 @@ struct ProcessState { int status_; }; +/// @brief ProcessStates container which stores a ProcessState for each process +/// identified by PID. typedef std::map<pid_t, ProcessState> ProcessStates; /// @brief Implementation of the @c ProcessSpawn class. /// -/// This pimpl idiom is used by the @c ProcessSpawn in this case to +/// This impl idiom is used by the @c ProcessSpawn in this case to /// avoid exposing the internals of the implementation, such as /// custom handling of a SIGCHLD signal, and the conversion of the /// arguments of the executable from the STL container to the array. @@ -60,7 +64,8 @@ public: /// @param executable A path to the program to be executed. /// @param args Arguments for the program to be executed. /// @param vars Environment variables for the program to be executed. - ProcessSpawnImpl(const std::string& executable, + ProcessSpawnImpl(IOServicePtr io_service, + const std::string& executable, const ProcessArgs& args, const ProcessEnvVars& vars); @@ -116,13 +121,6 @@ public: private: - /// @brief Access the single instance of the SignalSet which registers the - /// @ref waitForProcess function as the SIGCHLD signal handler. - /// - /// @return The single instance of the @ref SignalSet which handles the - /// SIGCHLD signal. - SignalSetPtr signalSet(); - /// @brief Copies the argument specified as a C++ string to the new /// C string. /// @@ -147,8 +145,8 @@ private: /// was a different signal. bool waitForProcess(int signum); - /// @brief A signal set installing a handler for SIGCHLD. - SignalSetPtr signals_; + /// @brief ASIO signal set. + IOSignalSetPtr io_signal_set_; /// @brief A map holding the status codes of executed processes. ProcessStates process_state_; @@ -167,13 +165,20 @@ private: /// @brief An storage container for all allocated C strings. std::vector<CStringPtr> storage_; + + /// @brief Mutex to protect internal state. + boost::shared_ptr<std::mutex> mutex_; }; -ProcessSpawnImpl::ProcessSpawnImpl(const std::string& executable, +ProcessSpawnImpl::ProcessSpawnImpl(IOServicePtr io_service, + const std::string& executable, const ProcessArgs& args, const ProcessEnvVars& vars) - : signals_(signalSet()), process_state_(), executable_(executable), - args_(new char*[args.size() + 2]), vars_(new char*[vars.size() + 1]) { + : executable_(executable), args_(new char*[args.size() + 2]), + vars_(new char*[vars.size() + 1]), mutex_(new std::mutex) { + io_signal_set_.reset(new IOSignalSet(io_service, + std::bind(&ProcessSpawnImpl::waitForProcess, + this, ph::_1))); // Conversion of the arguments to the C-style array we start by setting // all pointers within an array to NULL to indicate that they haven't // been allocated yet. @@ -194,22 +199,6 @@ ProcessSpawnImpl::ProcessSpawnImpl(const std::string& executable, ProcessSpawnImpl::~ProcessSpawnImpl() { } -SignalSetPtr -ProcessSpawnImpl::signalSet() { - static SignalSetPtr signals(new SignalSet(SIGCHLD)); - auto registerHandle = [&]() -> bool { - // Set the handler which is invoked immediately when the signal - // is received. - signals->setOnReceiptHandler(std::bind(&ProcessSpawnImpl::waitForProcess, - this, ph::_1)); - return (true); - }; - static bool init = registerHandle(); - (void)init; - - return (signals); -} - std::string ProcessSpawnImpl::getCommandLine() const { std::ostringstream s; @@ -258,8 +247,8 @@ ProcessSpawnImpl::spawn() { // We're in the parent process. try { - process_state_.insert( - std::pair<pid_t, ProcessState>(pid, ProcessState())); + lock_guard<std::mutex> lk(*mutex_); + process_state_.insert(std::pair<pid_t, ProcessState>(pid, ProcessState())); } catch(...) { pthread_sigmask(SIG_SETMASK, &osset, 0); throw; @@ -270,6 +259,7 @@ ProcessSpawnImpl::spawn() { bool ProcessSpawnImpl::isRunning(const pid_t pid) const { + lock_guard<std::mutex> lk(*mutex_); ProcessStates::const_iterator proc = process_state_.find(pid); if (proc == process_state_.end()) { isc_throw(BadValue, "the process with the pid '" << pid @@ -281,6 +271,7 @@ ProcessSpawnImpl::isRunning(const pid_t pid) const { bool ProcessSpawnImpl::isAnyRunning() const { + lock_guard<std::mutex> lk(*mutex_); for (ProcessStates::const_iterator proc = process_state_.begin(); proc != process_state_.end(); ++proc) { if (proc->second.running_) { @@ -292,6 +283,7 @@ ProcessSpawnImpl::isAnyRunning() const { int ProcessSpawnImpl::getExitStatus(const pid_t pid) const { + lock_guard<std::mutex> lk(*mutex_); ProcessStates::const_iterator proc = process_state_.find(pid); if (proc == process_state_.end()) { isc_throw(InvalidOperation, "the process with the pid '" << pid @@ -325,6 +317,7 @@ ProcessSpawnImpl::waitForProcess(int signum) { // after this signal handler does his work. int errno_value = errno; + lock_guard<std::mutex> lk(*mutex_); for (;;) { int status = 0; pid_t pid = waitpid(-1, &status, WNOHANG); @@ -359,13 +352,15 @@ ProcessSpawnImpl::clearState(const pid_t pid) { isc_throw(InvalidOperation, "unable to remove the status for the" "process (pid: " << pid << ") which is still running"); } + lock_guard<std::mutex> lk(*mutex_); process_state_.erase(pid); } -ProcessSpawn::ProcessSpawn(const std::string& executable, +ProcessSpawn::ProcessSpawn(IOServicePtr io_service, + const std::string& executable, const ProcessArgs& args, const ProcessEnvVars& vars) - : impl_(new ProcessSpawnImpl(executable, args, vars)) { + : impl_(new ProcessSpawnImpl(io_service, executable, args, vars)) { } ProcessSpawn::~ProcessSpawn() { @@ -401,5 +396,5 @@ ProcessSpawn::clearState(const pid_t pid) { return (impl_->clearState(pid)); } -} -} +} // namespace asiolink +} // namespace isc diff --git a/src/lib/util/process_spawn.h b/src/lib/asiolink/process_spawn.h index 3fb10669a8..b0b2971d0d 100644 --- a/src/lib/util/process_spawn.h +++ b/src/lib/asiolink/process_spawn.h @@ -7,6 +7,7 @@ #ifndef PROCESS_SPAWN_H #define PROCESS_SPAWN_H +#include <asiolink/io_service.h> #include <exceptions/exceptions.h> #include <boost/noncopyable.hpp> #include <string> @@ -15,7 +16,7 @@ #include <boost/shared_ptr.hpp> namespace isc { -namespace util { +namespace asiolink { /// @brief Exception thrown when error occurs during spawning a process. class ProcessSpawnError : public Exception { @@ -55,20 +56,18 @@ typedef std::vector<std::string> ProcessEnvVars; /// such as the SIGCHLD signal handler. In addition making it /// noncopyable keeps the static check code from flagging the /// lack of a copy constructor as an issue. -/// -/// @todo The SIGCHLD handling logic should be moved to the @c SignalSet -/// class so as multiple instances of the @c ProcessSpawn use the same -/// SIGCHLD signal handler. class ProcessSpawn : boost::noncopyable { public: /// @brief Constructor. /// + /// @param io_service The IOService which handles signal handlers. /// @param executable A path to the program to be executed. /// @param args Arguments for the program to be executed. /// @param vars Environment variables for the program to be executed. - ProcessSpawn(const std::string& executable, + ProcessSpawn(isc::asiolink::IOServicePtr io_service, + const std::string& executable, const ProcessArgs& args = ProcessArgs(), const ProcessEnvVars& vars = ProcessEnvVars()); @@ -141,7 +140,7 @@ private: ProcessSpawnImplPtr impl_; }; -} -} +} // namespace asiolink +} // namespace isc #endif // PROCESS_SPAWN_H diff --git a/src/lib/asiolink/tests/Makefile.am b/src/lib/asiolink/tests/Makefile.am index c1e48c27b6..4d3cea43c9 100644 --- a/src/lib/asiolink/tests/Makefile.am +++ b/src/lib/asiolink/tests/Makefile.am @@ -2,6 +2,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib AM_CPPFLAGS += $(BOOST_INCLUDES) AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(srcdir)/testdata\" AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_top_builddir)/src/lib/asiolink/tests\" +AM_CPPFLAGS += -DTEST_DATA_TOPBUILDDIR=\"$(abs_top_builddir)\" AM_CXXFLAGS = $(KEA_CXXFLAGS) @@ -11,6 +12,10 @@ endif CLEANFILES = *.gcno *.gcda test-socket +DISTCLEANFILES = process_spawn_app.sh + +noinst_SCRIPTS = process_spawn_app.sh + TESTS_ENVIRONMENT = \ $(LIBTOOL) --mode=execute $(VALGRIND_COMMAND) @@ -29,8 +34,10 @@ run_unittests_SOURCES += tcp_socket_unittest.cc run_unittests_SOURCES += udp_endpoint_unittest.cc run_unittests_SOURCES += udp_socket_unittest.cc run_unittests_SOURCES += io_service_unittest.cc +run_unittests_SOURCES += io_service_signal_unittests.cc run_unittests_SOURCES += dummy_io_callback_unittest.cc run_unittests_SOURCES += tcp_acceptor_unittest.cc +run_unittests_SOURCES += process_spawn_unittest.cc run_unittests_SOURCES += unix_domain_socket_unittest.cc run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) diff --git a/src/lib/process/tests/io_service_signal_unittests.cc b/src/lib/asiolink/tests/io_service_signal_unittests.cc index dbbe0cfbbb..7175d8f68b 100644 --- a/src/lib/process/tests/io_service_signal_unittests.cc +++ b/src/lib/asiolink/tests/io_service_signal_unittests.cc @@ -6,18 +6,21 @@ #include <config.h> -#include <process/io_service_signal.h> -#include <process/testutils/d_test_stubs.h> +#include <exceptions/exceptions.h> +#include <asiolink/interval_timer.h> +#include <asiolink/io_service_signal.h> +#include <asiolink/testutils/timed_signal.h> #include <gtest/gtest.h> #include <functional> #include <queue> +using namespace isc::asiolink::test; namespace ph = std::placeholders; namespace isc { -namespace process { +namespace asiolink { /// @brief Test fixture for testing the use of IO service signals. /// @@ -256,5 +259,5 @@ TEST_F(IOSignalTest, mixedSignals) { EXPECT_EQ(sigusr2_cnt, (stop_at_count_/3)); } -}; // end of isc::process namespace -}; // end of isc namespace +} // namespace asiolink +} // namespace isc diff --git a/src/lib/util/tests/process_spawn_app.sh.in b/src/lib/asiolink/tests/process_spawn_app.sh.in index 99997aa81c..99997aa81c 100644 --- a/src/lib/util/tests/process_spawn_app.sh.in +++ b/src/lib/asiolink/tests/process_spawn_app.sh.in diff --git a/src/lib/util/tests/process_spawn_unittest.cc b/src/lib/asiolink/tests/process_spawn_unittest.cc index 6c181a8437..8d12e7745d 100644 --- a/src/lib/util/tests/process_spawn_unittest.cc +++ b/src/lib/asiolink/tests/process_spawn_unittest.cc @@ -6,7 +6,7 @@ #include <config.h> -#include <util/process_spawn.h> +#include <asiolink/process_spawn.h> #include <gtest/gtest.h> #include <signal.h> #include <stdint.h> @@ -17,7 +17,7 @@ namespace { using namespace isc; -using namespace isc::util; +using namespace isc::asiolink; using namespace std; /// @brief Returns a location of the test script. @@ -98,7 +98,8 @@ bool waitForProcessFast(const ProcessSpawn& process, const pid_t pid, // case: fork() failing) TEST(ProcessSpawn, sigchldBlocked) { vector<string> args; - ProcessSpawn process(getApp(), args); + IOServicePtr io_service(new asiolink::IOService()); + ProcessSpawn process(io_service, getApp(), args); sigset_t sset; sigemptyset(&sset); sigaddset(&sset, SIGCHLD); @@ -114,7 +115,8 @@ TEST(ProcessSpawn, spawnWithArgs) { vector<string> args; args.push_back("-e"); args.push_back("64"); - ProcessSpawn process(getApp(), args); + IOServicePtr io_service(new asiolink::IOService()); + ProcessSpawn process(io_service, getApp(), args); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); @@ -133,7 +135,8 @@ TEST(ProcessSpawn, spawnWithArgsAndEnvVars) { args.push_back("TEST_VARIABLE_VALUE"); vars.push_back("TEST_VARIABLE_NAME=TEST_VARIABLE_VALUE"); - ProcessSpawn process(getApp(), args, vars); + IOServicePtr io_service(new asiolink::IOService()); + ProcessSpawn process(io_service, getApp(), args, vars); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); @@ -149,7 +152,8 @@ TEST(ProcessSpawn, spawnWithArgsAndEnvVars) { TEST(ProcessSpawn, spawnTwoProcesses) { vector<string> args; args.push_back("-p"); - ProcessSpawn process(getApp(), args); + IOServicePtr io_service(new asiolink::IOService()); + ProcessSpawn process(io_service, getApp(), args); pid_t pid1 = 0; ASSERT_NO_THROW(pid1 = process.spawn()); ASSERT_TRUE(waitForProcess(process, pid1, 2)); @@ -174,8 +178,9 @@ TEST(ProcessSpawn, spawnTwoProcesses) { // This test verifies that the external application can be ran without // arguments and that the exit code is gathered. TEST(ProcessSpawn, spawnNoArgs) { + IOServicePtr io_service(new asiolink::IOService()); vector<string> args; - ProcessSpawn process(getApp()); + ProcessSpawn process(io_service, getApp()); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); @@ -187,7 +192,8 @@ TEST(ProcessSpawn, spawnNoArgs) { // This test verifies that the EXIT_FAILURE code is returned when // application can't be executed. TEST(ProcessSpawn, invalidExecutable) { - ProcessSpawn process("foo"); + IOServicePtr io_service(new asiolink::IOService()); + ProcessSpawn process(io_service, "foo"); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); @@ -199,6 +205,7 @@ TEST(ProcessSpawn, invalidExecutable) { // This test verifies that the full command line for the process is // returned. TEST(ProcessSpawn, getCommandLine) { + IOServicePtr io_service(new asiolink::IOService()); // Note that cases below are enclosed in separate scopes to make // sure that the ProcessSpawn object is destroyed before a new // object is created. Current implementation doesn't allow for @@ -211,13 +218,13 @@ TEST(ProcessSpawn, getCommandLine) { args.push_back("-y"); args.push_back("foo"); args.push_back("bar"); - ProcessSpawn process("myapp", args); + ProcessSpawn process(io_service, "myapp", args); EXPECT_EQ("myapp -x -y foo bar", process.getCommandLine()); } { // Case 2: no arguments. - ProcessSpawn process("myapp"); + ProcessSpawn process(io_service, "myapp"); EXPECT_EQ("myapp", process.getCommandLine()); } } @@ -225,12 +232,13 @@ TEST(ProcessSpawn, getCommandLine) { // This test verifies that it is possible to check if the process is // running. TEST(ProcessSpawn, isRunning) { + IOServicePtr io_service(new asiolink::IOService()); // Run the process which sleeps for 10 seconds, so as we have // enough time to check if it is running. vector<string> args; args.push_back("-s"); args.push_back("10"); - ProcessSpawn process(getApp(), args); + ProcessSpawn process(io_service, getApp(), args); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); EXPECT_TRUE(process.isRunning(pid)); @@ -244,7 +252,7 @@ TEST(ProcessSpawn, isRunning) { // This test verifies that the signal handler does not modify value of // errno. TEST(ProcessSpawn, errnoInvariance) { - + IOServicePtr io_service(new asiolink::IOService()); // Set errno to an arbitrary value. We'll later check that it was not // stumped on. errno = 123; @@ -252,7 +260,7 @@ TEST(ProcessSpawn, errnoInvariance) { vector<string> args; args.push_back("-e"); args.push_back("64"); - ProcessSpawn process(getApp(), args); + ProcessSpawn process(io_service, getApp(), args); pid_t pid = 0; ASSERT_NO_THROW(pid = process.spawn()); diff --git a/src/lib/asiolink/testutils/Makefile.am b/src/lib/asiolink/testutils/Makefile.am index ec4d1815b8..a20cae5432 100644 --- a/src/lib/asiolink/testutils/Makefile.am +++ b/src/lib/asiolink/testutils/Makefile.am @@ -12,6 +12,7 @@ if HAVE_GTEST noinst_LTLIBRARIES = libasiolinktest.la libasiolinktest_la_SOURCES = test_server_unix_socket.cc test_server_unix_socket.h +libasiolinktest_la_SOURCES += timed_signal.cc timed_signal.h libasiolinktest_la_CXXFLAGS = $(AM_CXXFLAGS) libasiolinktest_la_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES) diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 2891fa866d..da1b31c61b 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include <config.h> + #include <database/database_connection.h> #include <dhcpsrv/cfgmgr.h> #include <dhcpsrv/dhcpsrv_exceptions.h> @@ -15,8 +16,6 @@ #include <exceptions/exceptions.h> #include <util/multi_threading_mgr.h> #include <util/pid_file.h> -#include <util/process_spawn.h> -#include <util/signal_set.h> #include <cstdio> #include <cstring> #include <errno.h> @@ -106,7 +105,7 @@ private: /// @brief A pointer to the @c ProcessSpawn object used to execute /// the LFC. - boost::scoped_ptr<util::ProcessSpawn> process_; + boost::scoped_ptr<ProcessSpawn> process_; /// @brief A pointer to the callback function executed by the timer. asiolink::IntervalTimer::Callback callback_; @@ -174,7 +173,7 @@ LFCSetup::setup(const uint32_t lfc_interval, lease_file6->getFilename(); // Create the other names by appending suffixes to the base name. - util::ProcessArgs args; + ProcessArgs args; // Universe: v4 or v6. args.push_back(lease_file4 ? "-4" : "-6"); @@ -204,7 +203,7 @@ LFCSetup::setup(const uint32_t lfc_interval, args.push_back("ignored-path"); // Create the process (do not start it yet). - process_.reset(new util::ProcessSpawn(executable, args)); + process_.reset(new ProcessSpawn(LeaseMgr::getIOService(), executable, args)); // If we've been told to run it once now, invoke the callback directly. if (run_once_now) { @@ -630,7 +629,7 @@ const int Memfile_LeaseMgr::MAJOR_VERSION; const int Memfile_LeaseMgr::MINOR_VERSION; Memfile_LeaseMgr::Memfile_LeaseMgr(const DatabaseConnection::ParameterMap& parameters) - : LeaseMgr(), lfc_setup_(), conn_(parameters), mutex_() { + : LeaseMgr(), lfc_setup_(), conn_(parameters), mutex_(new std::mutex) { bool conversion_needed = false; // Check the universe and use v4 file or v6 file. @@ -666,8 +665,6 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const DatabaseConnection::ParameterMap& param } lfcSetup(conversion_needed); } - - mutex_.reset(new std::mutex); } Memfile_LeaseMgr::~Memfile_LeaseMgr() { diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index 9e7ce5e986..f1dd01f215 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -8,13 +8,13 @@ #define MEMFILE_LEASE_MGR_H #include <asiolink/interval_timer.h> +#include <asiolink/process_spawn.h> #include <database/database_connection.h> #include <dhcp/hwaddr.h> #include <dhcpsrv/csv_lease_file4.h> #include <dhcpsrv/csv_lease_file6.h> #include <dhcpsrv/memfile_lease_storage.h> #include <dhcpsrv/lease_mgr.h> -#include <util/process_spawn.h> #include <boost/scoped_ptr.hpp> #include <boost/shared_ptr.hpp> diff --git a/src/lib/process/Makefile.am b/src/lib/process/Makefile.am index 33db11654d..9a07cb7c68 100644 --- a/src/lib/process/Makefile.am +++ b/src/lib/process/Makefile.am @@ -25,7 +25,6 @@ libkea_process_la_SOURCES += d_controller.cc d_controller.h libkea_process_la_SOURCES += d_log.cc d_log.h libkea_process_la_SOURCES += d_process.h libkea_process_la_SOURCES += daemon.cc daemon.h -libkea_process_la_SOURCES += io_service_signal.cc io_service_signal.h libkea_process_la_SOURCES += log_parser.cc log_parser.h libkea_process_la_SOURCES += logging_info.cc logging_info.h libkea_process_la_SOURCES += process_messages.cc process_messages.h @@ -93,7 +92,6 @@ libkea_process_include_HEADERS = \ d_controller.h \ d_log.h \ d_process.h \ - io_service_signal.h \ logging_info.h \ log_parser.h \ process_messages.h diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index 176967ea67..72bf80e027 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -18,7 +18,9 @@ #include <functional> #include <sstream> #include <unistd.h> +#include <signal.h> +using namespace isc::asiolink; using namespace isc::data; using namespace isc::config; namespace ph = std::placeholders; @@ -231,8 +233,7 @@ DControllerBase::checkConfigOnly() { } void -DControllerBase::parseArgs(int argc, char* argv[]) -{ +DControllerBase::parseArgs(int argc, char* argv[]) { if (argc == 1) { isc_throw(InvalidUsage, ""); @@ -312,8 +313,7 @@ DControllerBase::parseArgs(int argc, char* argv[]) } bool -DControllerBase::customOption(int /* option */, char* /*optarg*/) -{ +DControllerBase::customOption(int /* option */, char* /*optarg*/) { // Default implementation returns false. return (false); } @@ -783,8 +783,7 @@ DControllerBase::processSignal(int signum) { } void -DControllerBase::usage(const std::string & text) -{ +DControllerBase::usage(const std::string & text) { if (text != "") { std::cerr << "Usage error: " << text << std::endl; } diff --git a/src/lib/process/d_controller.h b/src/lib/process/d_controller.h index 4598288111..d13b30dbc7 100644 --- a/src/lib/process/d_controller.h +++ b/src/lib/process/d_controller.h @@ -1,4 +1,4 @@ -// Copyright (C) 2013-2020 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -8,13 +8,13 @@ #define D_CONTROLLER_H #include <asiolink/io_service.h> +#include <asiolink/io_service_signal.h> #include <cc/data.h> #include <exceptions/exceptions.h> #include <log/logger_support.h> #include <process/daemon.h> #include <process/d_log.h> #include <process/d_process.h> -#include <process/io_service_signal.h> #include <boost/shared_ptr.hpp> #include <boost/noncopyable.hpp> @@ -635,10 +635,10 @@ private: DProcessBasePtr process_; /// @brief Shared pointer to an IOService object, used for ASIO operations. - asiolink::IOServicePtr io_service_; + isc::asiolink::IOServicePtr io_service_; /// @brief ASIO signal set. - IOSignalSetPtr io_signal_set_; + isc::asiolink::IOSignalSetPtr io_signal_set_; /// @brief Singleton instance value. static DControllerBasePtr controller_; @@ -648,7 +648,7 @@ private: friend class DControllerTest; }; -}; // namespace isc::process -}; // namespace isc +} // namespace isc::process +} // namespace isc #endif diff --git a/src/lib/process/daemon.cc b/src/lib/process/daemon.cc index a7dba8af38..3757570843 100644 --- a/src/lib/process/daemon.cc +++ b/src/lib/process/daemon.cc @@ -36,7 +36,7 @@ std::string Daemon::proc_name_(""); std::string Daemon::default_logger_name_("kea"); Daemon::Daemon() - : signal_set_(), signal_handler_(), config_file_(""), + : signal_set_(), config_file_(""), pid_file_dir_(DATA_DIR), pid_file_(), am_file_author_(false), exit_value_(EXIT_SUCCESS) { @@ -55,18 +55,11 @@ Daemon::~Daemon() { } void Daemon::cleanup() { - } void Daemon::shutdown() { } -void Daemon::handleSignal() { - if (signal_set_ && signal_handler_) { - signal_set_->handleNext(std::bind(signal_handler_, ph::_1)); - } -} - void Daemon::configureLogger(const ConstElementPtr& log_config, const ConfigPtr& storage) { diff --git a/src/lib/process/daemon.h b/src/lib/process/daemon.h index 5c9b4260fa..f3a9e51aaf 100644 --- a/src/lib/process/daemon.h +++ b/src/lib/process/daemon.h @@ -10,7 +10,7 @@ #include <cc/data.h> #include <process/config_base.h> #include <util/pid_file.h> -#include <util/signal_set.h> +#include <asiolink/io_service_signal.h> #include <boost/noncopyable.hpp> #include <boost/date_time/posix_time/posix_time.hpp> #include <string> @@ -36,8 +36,8 @@ public: /// implementations should derive from it. /// /// Methods are not pure virtual, as we need to instantiate basic daemons (e.g. -/// Dhcpv6Srv) in tests, without going through the hassles of implementing stub -/// methods. +/// Dhcpv4Srv and Dhcpv6Srv) in tests, without going through the hassles of +/// implementing stub methods. /// /// Classes derived from @c Daemon may install custom signal handlers using /// @c isc::util::SignalSet class. This base class provides a declaration @@ -232,31 +232,12 @@ public: protected: - /// @brief Invokes handler for the next received signal. - /// - /// This function provides a default implementation for the function - /// handling next signal received by the process. It checks if a pointer - /// to @c isc::util::SignalSet object and the signal handler function - /// have been set. If they have been set, the signal handler is invoked for - /// the the next signal registered in the @c SignalSet object. - /// - /// This function should be received in the main loop of the process. - virtual void handleSignal(); - /// @brief A pointer to the object installing custom signal handlers. /// - /// This pointer needs to be initialized to point to the @c SignalSet + /// This pointer needs to be initialized to point to the @c IOSignalSet /// object in the derived classes which need to handle signals received /// by the process. - isc::util::SignalSetPtr signal_set_; - - /// @brief Pointer to the common signal handler invoked by the handleSignal - /// function. - /// - /// This pointer needs to be initialized to point to the signal handler - /// function for signals being handled by the process. If signal handler - /// it not initialized, the signals will not be handled. - isc::util::SignalHandler signal_handler_; + isc::asiolink::IOSignalSetPtr signal_set_; /// @brief Manufacture the pid file name std::string makePIDFileName() const; @@ -292,7 +273,7 @@ private: int exit_value_; }; -}; // end of isc::dhcp namespace -}; // end of isc namespace +} // namespace process +} // namespace isc #endif diff --git a/src/lib/process/process_messages.cc b/src/lib/process/process_messages.cc index f6dd5e145a..af290ed409 100644 --- a/src/lib/process/process_messages.cc +++ b/src/lib/process/process_messages.cc @@ -33,7 +33,6 @@ extern const isc::log::MessageID DCTL_RUN_PROCESS = "DCTL_RUN_PROCESS"; extern const isc::log::MessageID DCTL_SESSION_FAIL = "DCTL_SESSION_FAIL"; 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_SIGNAL_ERROR = "DCTL_SIGNAL_ERROR"; extern const isc::log::MessageID DCTL_STANDALONE = "DCTL_STANDALONE"; extern const isc::log::MessageID DCTL_STARTING = "DCTL_STARTING"; extern const isc::log::MessageID DCTL_UNSUPPORTED_SIGNAL = "DCTL_UNSUPPORTED_SIGNAL"; @@ -70,7 +69,6 @@ const char* values[] = { "DCTL_SESSION_FAIL", "%1 controller failed to establish Kea session: %1", "DCTL_SHUTDOWN", "%1 has shut down, pid: %2, version: %3", "DCTL_SHUTDOWN_SIGNAL_RECVD", "OS signal %1 received, starting shutdown", - "DCTL_SIGNAL_ERROR", "signal handler for signal %1, threw an unexpected exception: %2", "DCTL_STANDALONE", "%1 skipping message queue, running standalone", "DCTL_STARTING", "%1 starting, pid: %2, version: %3 (%4)", "DCTL_UNSUPPORTED_SIGNAL", "ignoring reception of unsupported signal: %1", diff --git a/src/lib/process/process_messages.h b/src/lib/process/process_messages.h index bb96e6a899..8e1334ea98 100644 --- a/src/lib/process/process_messages.h +++ b/src/lib/process/process_messages.h @@ -34,7 +34,6 @@ extern const isc::log::MessageID DCTL_RUN_PROCESS; extern const isc::log::MessageID DCTL_SESSION_FAIL; extern const isc::log::MessageID DCTL_SHUTDOWN; extern const isc::log::MessageID DCTL_SHUTDOWN_SIGNAL_RECVD; -extern const isc::log::MessageID DCTL_SIGNAL_ERROR; extern const isc::log::MessageID DCTL_STANDALONE; extern const isc::log::MessageID DCTL_STARTING; extern const isc::log::MessageID DCTL_UNSUPPORTED_SIGNAL; diff --git a/src/lib/process/process_messages.mes b/src/lib/process/process_messages.mes index 00800b1772..2b5873601d 100644 --- a/src/lib/process/process_messages.mes +++ b/src/lib/process/process_messages.mes @@ -132,12 +132,6 @@ down. The argument specifies a name of the service. This is a debug message indicating the application has received a signal instructing it to shutdown. -% DCTL_SIGNAL_ERROR signal handler for signal %1, threw an unexpected exception: %2 -This is an error message indicating that the application encountered an unexpected -error after receiving a signal. This is a programmatic error and should be -reported. While The application will likely continue to operating, it may be -unable to respond correctly to signals. - % DCTL_STANDALONE %1 skipping message queue, running standalone This is a debug message indicating that the controller is running in the application in standalone mode. This means it will not connected to the Kea diff --git a/src/lib/process/tests/Makefile.am b/src/lib/process/tests/Makefile.am index 2fb87683e6..7ef567359b 100644 --- a/src/lib/process/tests/Makefile.am +++ b/src/lib/process/tests/Makefile.am @@ -28,7 +28,6 @@ libprocess_unittests_SOURCES += config_ctl_info_unittests.cc libprocess_unittests_SOURCES += config_ctl_parser_unittests.cc libprocess_unittests_SOURCES += d_controller_unittests.cc libprocess_unittests_SOURCES += daemon_unittest.cc -libprocess_unittests_SOURCES += io_service_signal_unittests.cc libprocess_unittests_SOURCES += log_parser_unittests.cc libprocess_unittests_SOURCES += logging_info_unittests.cc libprocess_unittests_SOURCES += run_unittests.cc diff --git a/src/lib/process/tests/d_controller_unittests.cc b/src/lib/process/tests/d_controller_unittests.cc index 010cd4e914..f1cedb1006 100644 --- a/src/lib/process/tests/d_controller_unittests.cc +++ b/src/lib/process/tests/d_controller_unittests.cc @@ -7,6 +7,7 @@ #include <config.h> #include <kea_version.h> +#include <asiolink/testutils/timed_signal.h> #include <cc/command_interpreter.h> #include <process/testutils/d_test_stubs.h> @@ -15,6 +16,7 @@ #include <sstream> +using namespace isc::asiolink::test; using namespace boost::posix_time; namespace isc { diff --git a/src/lib/process/testutils/d_test_stubs.h b/src/lib/process/testutils/d_test_stubs.h index 11635e695d..ee3b0c9d21 100644 --- a/src/lib/process/testutils/d_test_stubs.h +++ b/src/lib/process/testutils/d_test_stubs.h @@ -721,69 +721,7 @@ public: isc::data::ConstElementPtr answer_; }; -/// @brief Implements a time-delayed signal -/// -/// Given an IOService, a signal number, and a time period, this class will -/// send (raise) the signal to the current process. -class TimedSignal { -public: - /// @brief Constructor - /// - /// @param io_service IOService to run the timer - /// @param signum OS signal value (e.g. SIGHUP, SIGUSR1 ...) - /// @param milliseconds time in milliseconds to wait until the signal is - /// raised. - /// @param mode selects between a one-shot signal or a signal which repeats - /// at "milliseconds" interval. - TimedSignal(asiolink::IOService& io_service, int signum, int milliseconds, - const asiolink::IntervalTimer::Mode& mode = - asiolink::IntervalTimer::ONE_SHOT) - : timer_(new asiolink::IntervalTimer(io_service)) { - timer_->setup(SendSignalCallback(signum), milliseconds, mode); - } - - /// @brief Cancels the given timer. - void cancel() { - if (timer_) { - timer_->cancel(); - } - } - - /// @brief Destructor. - ~TimedSignal() { - cancel(); - } - - /// @brief Callback for the TimeSignal's internal timer. - class SendSignalCallback: public std::unary_function<void, void> { - public: - - /// @brief Constructor - /// - /// @param signum OS signal value of the signal to send - SendSignalCallback(int signum) : signum_(signum) { - } - - /// @brief Callback method invoked when the timer expires - /// - /// Calls raise with the given signal which should generate that - /// signal to the given process. - void operator()() { - ASSERT_EQ(0, raise(signum_)); - return; - } - - private: - /// @brief Stores the OS signal value to send. - int signum_; - }; - -private: - /// @brief Timer which controls when the signal is sent. - asiolink::IntervalTimerPtr timer_; -}; - -}; // namespace isc::process -}; // namespace isc +} // namespace isc::process +} // namespace isc #endif diff --git a/src/lib/util/Makefile.am b/src/lib/util/Makefile.am index cee9b62d65..ac2ca57401 100644 --- a/src/lib/util/Makefile.am +++ b/src/lib/util/Makefile.am @@ -21,10 +21,8 @@ libkea_util_la_SOURCES += multi_threading_mgr.h multi_threading_mgr.cc libkea_util_la_SOURCES += optional.h libkea_util_la_SOURCES += pid_file.h pid_file.cc libkea_util_la_SOURCES += pointer_util.h -libkea_util_la_SOURCES += process_spawn.h process_spawn.cc libkea_util_la_SOURCES += range_utilities.h libkea_util_la_SOURCES += readwrite_mutex.h -libkea_util_la_SOURCES += signal_set.cc signal_set.h libkea_util_la_SOURCES += staged_value.h libkea_util_la_SOURCES += state_model.cc state_model.h libkea_util_la_SOURCES += stopwatch.cc stopwatch.h @@ -71,10 +69,8 @@ libkea_util_include_HEADERS = \ optional.h \ pid_file.h \ pointer_util.h \ - process_spawn.h \ range_utilities.h \ readwrite_mutex.h \ - signal_set.h \ staged_value.h \ state_model.h \ stopwatch.h \ diff --git a/src/lib/util/signal_set.cc b/src/lib/util/signal_set.cc deleted file mode 100644 index f0695ea13e..0000000000 --- a/src/lib/util/signal_set.cc +++ /dev/null @@ -1,415 +0,0 @@ -// Copyright (C) 2014-2020 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include <config.h> - -#include <util/signal_set.h> - -#include <array> -#include <cerrno> -#include <cstring> -#include <list> - -using namespace isc; -using namespace isc::util; - -namespace { - -/// @brief Fixed size storage for the codes of received signals. -/// -/// It is initialized with all zeros, which means that no signals -/// have been received. -std::array<int, 1024> received_signals; - -/// @brief Returns a pointer to the global set of registered signals. -/// -/// Multiple instances of @c SignalSet may use this pointer to access -/// and update the set. Note we use a smart pointer so callers can ensure -/// the static object doesn't go out of scope before they are done with it -/// during process exit. -/// -/// @return Pointer to the global set of registered signals. This pointer -/// is always initialized and points to a valid object. -SigIntSetPtr getRegisteredSignals() { - static SigIntSetPtr registered_signals(new SigIntSet()); - return (registered_signals); -} - -/// @brief Returns a pointer to static collection of signals received. -/// -/// Multiple instances of @c SignalSet may use this pointer to access -/// and update the queue of signals received. Note we use a smart pointer -/// so callers can ensure the static object doesn't go out of scope before -/// they are done with it during process exit. -/// -/// @return Static collection of signals received. This pointer is always -/// initialized and points to a valid object. -SigIntListPtr getSignalStates() { - static SigIntListPtr signal_states(new SigIntList()); - return (signal_states); -} - -/// @brief Internal signal handler for @c isc::util::SignalSet class. -/// -/// This handler catches all registered signals. When a signal arrives it -/// passes the signal to invokeOnReceiptHandler for "on-receipt" processing. -/// If this processing returns true if exists without further action, -/// otherwise it adds the signal number to the queue of received signals. -/// It prevents adding duplicated signals. All duplicated signals are dropped. -/// This prevents hammering a process to invoke handlers (e.g. DHCP server -/// reconfiguration), when many of the same signals are received one after -/// another. -/// -/// @param sig Signal number. -void internalHandler(int sig) { - if (SignalSet::invokeOnReceiptHandler(sig)) { - // Signal has been handled by the on-receipt handler. - return; - } - - // Signal is using post-receipt handling, see if we've - // already received it. - SigIntListPtr states = getSignalStates(); - for (std::list<int>::const_iterator it = states->begin(); - it != states->end(); ++it) { - if (sig == *it) { - return; - } - } - - // We haven't yet received this signal so we have to record its - // reception. We're using the fixed size array to temporarily - // store the received signal codes. We can't directly insert - // the signal to the 'states' list because this would cause a - // new allocation (malloc) which is not async-signal-safe. - for (int i = 0; i < received_signals.size(); ++i) { - // We have already recorded this signal. - if (received_signals[i] == sig) { - return; - - } else if (received_signals[i] == 0) { - // We reached the end of the useful data in the storage. - // Put the signal code into the storage. - received_signals[i] = sig; - break; - } - } -} - -/// @brief Optional handler to execute at the time of signal receipt -BoolSignalHandler onreceipt_handler_ = BoolSignalHandler(); - -}; // end anon namespace - -namespace isc { -namespace util { - -bool -SignalSet::invokeOnReceiptHandler(int sig) { - if (!onreceipt_handler_) { - return (false); - } - - // First we set the signal to SIG_IGN. This causes any repeat occurrences - // to be discarded, not deferred as they would be if blocked. Note that - // we save the current sig action so we can restore it later. - struct sigaction sa; - struct sigaction prev_sa; - memset(&sa, 0, sizeof(sa)); - sa.sa_handler = SIG_IGN; - if (sigaction(sig, &sa, &prev_sa) < 0) { - // Highly unlikely we can get here. - const char* errmsg = strerror(errno); - isc_throw(SignalSetError, "failed to set SIG_IGN for signal " - << sig << ": " << errmsg); - } - - // Call the registered handler. - bool signal_processed = false; - try { - signal_processed = onreceipt_handler_(sig); - } catch (const std::exception& ex) { - // Restore the handler. We might fail to restore it, but we likely - // have bigger issues anyway: for that reason, the return value is - // ignored. To avoid complaints from static code checkers that notice - // that the return values from other calls to sigaction() have been - // used, the call to sigaction is explicitly cast to void to indicate - // that the return value is intentionally being ignored. - static_cast<void>(sigaction(sig, &prev_sa, 0)); - isc_throw(SignalSetError, "onreceipt_handler failed for signal " - << sig << ": " << ex.what()); - } - - // Restore the sig action to re-enable handling this signal. - if (sigaction(sig, &prev_sa, 0) < 0) { - // Highly unlikely we can get here. - const char* errmsg = strerror(errno); - isc_throw(SignalSetError, "failed to restore handler for signal " - << sig << ": " << errmsg); - } - - return (signal_processed); -} - -SignalSet::SignalSet(const int sig0) - : blocked_(false) { - // Copy static pointers to ensure they don't lose scope before we do. - registered_signals_ = getRegisteredSignals(); - signal_states_ = getSignalStates(); - add(sig0); -} - -SignalSet::SignalSet(const int sig0, const int sig1) : - blocked_(false) { - registered_signals_ = getRegisteredSignals(); - signal_states_ = getSignalStates(); - add(sig0); - add(sig1); -} - -SignalSet::SignalSet(const int sig0, const int sig1, const int sig2) : - blocked_(false) { - registered_signals_ = getRegisteredSignals(); - signal_states_ = getSignalStates(); - add(sig0); - add(sig1); - add(sig2); -} - -SignalSet::~SignalSet() { - // Set default signal handlers. - try { - clear(); - } catch (...) { - // Not a good thing to throw from a destructor. in fact this should - // not throw an exception because we just unregister the signals - // that we have previously registered. So the signal codes are fine. - } -} - -void -SignalSet::add(const int sig) { - insert(sig); - struct sigaction sa; - memset(&sa, 0, sizeof(sa)); - sa.sa_handler = internalHandler; - sigfillset(&sa.sa_mask); - if (sigaction(sig, &sa, 0) < 0) { - const char* errmsg = strerror(errno); - erase(sig); - isc_throw(SignalSetError, "failed to register a signal handler for" - " signal " << sig << ": " << errmsg); - } -} - -void -SignalSet::block() { - maskSignals(SIG_BLOCK); - blocked_ = true; -} - -void -SignalSet::clear() { - // Iterate over a copy of the registered signal set because the - // remove function is erasing the elements and we don't want to - // erase the elements we are iterating over. This would cause - // a segfault. - std::set<int> all_signals = local_signals_; - for (std::set<int>::const_iterator it = all_signals.begin(); - it != all_signals.end(); ++it) { - remove(*it); - } -} - -int -SignalSet::getNext() { - // Since we're operating on the global values that the signal - // handler has access to, we have to temporarily block the - // signals to not interfere with unsafe operations performed - // by this method. Remember if the signals were blocked or - // not to be able to determine whether they should be unblocked - // when the function returns. - bool was_blocked = blocked_; - if (!was_blocked) { - block(); - } - - // We may have some signals received. We need to iterate over those - // and move them to the signal states structure. - for (int i = 0; i < received_signals.size(); ++i) { - // If we hit the end of useful data, break. - if (received_signals[i] == 0) { - break; - } - // Check if such signal has been already recorded. - if (std::find(signal_states_->begin(), signal_states_->end(), received_signals[i]) - == signal_states_->end()) { - signal_states_->push_back(received_signals[i]); - } - // Set the current element to 0 to indicate the end of - // the useful data and process the next element. - received_signals[i] = 0; - } - - int sig = -1; - for (std::list<int>::iterator it = signal_states_->begin(); - it != signal_states_->end(); ++it) { - if (local_signals_.find(*it) != local_signals_.end()) { - sig = *it; - break; - } - } - - // If we blocked signals here locally, we have to unblock them. - if (!was_blocked) { - unblock(); - } - return (sig); -} - -void -SignalSet::erase(const int sig) { - if (local_signals_.find(sig) == local_signals_.end()) { - isc_throw(SignalSetError, "failed to unregister signal " << sig - << " from a signal set: signal is not owned by the" - " signal set"); - } - - // Since we're operating on the global values that the signal - // handler has access to, we have to temporarily block the - // signals to not interfere with unsafe operations performed - // by this method. Remember if the signals were blocked or - // not to be able to determine whether they should be unblocked - // when the function returns. - bool was_blocked = blocked_; - if (!was_blocked) { - block(); - } - - // Remove globally registered signal. - registered_signals_->erase(sig); - // Remove unhandled signals from the queue. - for (std::list<int>::iterator it = signal_states_->begin(); - it != signal_states_->end(); ++it) { - if (*it == sig) { - it = signal_states_->erase(it); - } - } - - // Remove locally registered signal. - local_signals_.erase(sig); - - // If the signal is no longer supported by this signal set, - // we have to unblock it, because other signal set instance - // may be using it. We won't process it anyway after it is - // removed from this set. - unblock(sig); - - // Unblock all remaining signals if we have blocked them in - // this function. - if (!was_blocked) { - unblock(); - } -} - -void -SignalSet::handleNext(SignalHandler signal_handler) { - block(); - int signum = getNext(); - if (signum >= 0) { - popNext(); - try { - signal_handler(signum); - } catch (...) { - unblock(); - throw; - } - } - unblock(); -} - -void -SignalSet::insert(const int sig) { - if ((registered_signals_->find(sig) != registered_signals_->end()) || - (local_signals_.find(sig) != local_signals_.end())) { - isc_throw(SignalSetError, "attempt to register a duplicate signal " - << sig); - } - registered_signals_->insert(sig); - local_signals_.insert(sig); -} - -void -SignalSet::maskSignals(const int mask, const int sig) const { - sigset_t new_set; - sigemptyset(&new_set); - if (sig < 0) { - for (std::set<int>::const_iterator it = registered_signals_->begin(); - it != registered_signals_->end(); ++it) { - sigaddset(&new_set, *it); - } - } else { - sigaddset(&new_set, sig); - } - pthread_sigmask(mask, &new_set, 0); -} - -void -SignalSet::popNext() { - for (std::list<int>::iterator it = signal_states_->begin(); - it != signal_states_->end(); ++it) { - if (local_signals_.find(*it) != local_signals_.end()) { - signal_states_->erase(it); - return; - } - } -} - -void -SignalSet::remove(const int sig) { - // Unregister only if we own this signal. - if (local_signals_.find(sig) != local_signals_.end()) { - struct sigaction sa; - memset(&sa, 0, sizeof(sa)); - sa.sa_handler = SIG_DFL; - sigfillset(&sa.sa_mask); - if (sigaction(sig, &sa, 0) < 0) { - isc_throw(SignalSetError, "unable to restore original signal" - " handler for signal: " << sig); - } - erase(sig); - } else { - isc_throw(SignalSetError, "failed to unregister signal " << sig - << ": this signal is not owned by the signal set"); - } -} - -void -SignalSet::unblock(int sig) { - // There are two cases supported by this function. One is to - // unblock some specific signal. Another one is to unblock all - // signals supported by this function. The maskSignals function - // will deal wih this internally but we marked signals as unblocked - // only if we unblocked all of them (sig is negative). - maskSignals(SIG_UNBLOCK, sig); - if (sig < 0) { - blocked_ = false; - } -} - - -void -SignalSet::setOnReceiptHandler(BoolSignalHandler handler) { - onreceipt_handler_ = handler; -} - -void -SignalSet::clearOnReceiptHandler() { - onreceipt_handler_ = BoolSignalHandler(); -} - -} // end of isc::util -} // end of isc diff --git a/src/lib/util/signal_set.h b/src/lib/util/signal_set.h deleted file mode 100644 index ab63276202..0000000000 --- a/src/lib/util/signal_set.h +++ /dev/null @@ -1,258 +0,0 @@ -// Copyright (C) 2014-2020 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#ifndef SIGNAL_SET_H -#define SIGNAL_SET_H - -#include <exceptions/exceptions.h> -#include <boost/noncopyable.hpp> -#include <boost/shared_ptr.hpp> -#include <functional> -#include <set> -#include <list> -#include <pthread.h> -#include <signal.h> - -namespace isc { -namespace util { - -/// @brief Exception thrown when the @c isc::util::SignalSet class -/// experiences an error. -class SignalSetError : public Exception { -public: - SignalSetError(const char* file, size_t line, const char* what) : - isc::Exception(file, line, what) { }; -}; - - -/// @brief Defines a set of integer signal identifiers: SIGHUP, SIGTERM... -typedef std::set<int> SigIntSet; -/// @brief Pointer to a set of signal identifiers -typedef boost::shared_ptr<SigIntSet> SigIntSetPtr; - -/// @brief Defines a list of integer signal identifiers: SIGHUP, SIGTERM... -typedef std::list<int> SigIntList; -/// @brief Pointer to a list of signal identifiers -typedef boost::shared_ptr<SigIntList> SigIntListPtr; - - -/// @brief Forward declaration to the @c isc::util::SignalSet. -class SignalSet; -/// @brief Pointer to the @c isc::util::SignalSet. -typedef boost::shared_ptr<SignalSet> SignalSetPtr; -/// @brief Pointer to the signal handling function. -typedef std::function<void(int signum)> SignalHandler; - -/// @brief Pointer to a signal handling function which returns bool result. -/// -/// The handler is expected to return true if the signal it was given has -/// been processed (i.e. should not be recorded for deferred processing) or -/// false in which case it will be recorded. -typedef std::function<bool(int signum)> BoolSignalHandler; - -/// @brief Represents a collection of signals handled in a customized way. -/// -/// Kea processes must handle selected signals in a specialized way. For -/// example: SIGINT and SIGTERM must perform a graceful shut down of the -/// server. The SIGHUP signal is used to trigger server's reconfiguration. -/// -/// This class allows the caller to register one or more signals to catch -/// and process. Signals may be handled either immediately upon arrival and/or -/// recorded and processed later. To process signals immediately, the caller -/// must register an "on-receipt" handler. This handler is expected to return -/// a true or false indicating whether or not the signal has been processed. -/// Signal occurrences that are not processed by the on-receipt handler are -/// remembered by SignalSet for deferred processing. The caller can then query -/// SignalSet at their discretion to determine if any signal occurrences are -/// pending and process them. -/// -/// SignalSet uses an internal handler to catch all registered signals. When -/// a signal arrives the internal handler will first attempt to invoke the -/// on-receipt handler. If one has been registered it is passed the -/// signal value as an argument and if it returns true upon completion, the -/// internal handler will exit without further action. If the on-receipt -/// handler returned false or one is not registered, then internal handler -/// will record the signal value for deferred processing. Note that once a -/// particular signal has been recorded, any further occurrences of that signal -/// will be discarded until the original occurrence has been processed. This -/// guards against rapid-fire occurrences of the same signal. -/// -/// @note This class is not thread safe. It uses static variables and -/// functions to track a global state of signal registration and received -/// signals' queue. But the thread library is signal safe as new threads -/// are created with all signals blocked. -class SignalSet : public boost::noncopyable { -public: - /// @brief Constructor installing one signal. - /// - /// @param sig0 First signal. - /// @throw SignalSetError If attempting to add duplicated signal or - /// the signal is invalid. - SignalSet(const int sig0); - - /// @brief Constructor installing two signals. - /// - /// @param sig0 First signal. - /// @param sig1 Second signal. - /// @throw SignalSetError If attempting to add duplicated signal or - /// the signal is invalid. - SignalSet(const int sig0, const int sig1); - - /// @brief Constructor installing three signals. - /// - /// @param sig0 First signal. - /// @param sig1 Second signal. - /// @param sig2 Third signal. - /// @throw SignalSetError If attempting to add duplicated signal or - /// the signal is invalid. - SignalSet(const int sig0, const int sig1, const int sig2); - - /// @brief Destructor. - /// - /// Removes installed handlers. - ~SignalSet(); - - /// @brief Installs the handler for the specified signal. - /// - /// This function adds a signal to the set. When the signal is received - /// by the process, it will be recorded and a signal can be later handled - /// by the process. - /// - /// @param sig Signal code. - /// @throw SignalSetError if signal being added duplicates an existing - /// signal. - void add(const int sig); - - /// @brief Uninstalls all signals. - /// - /// This function calls @c isc::util::SignalSet::remove for each - /// installed signal. - void clear(); - - /// @brief Returns a code of the next received signal. - /// - /// @return A code of the next received signal or -1 if there are no - /// more signals received. - int getNext(); - - /// @brief Calls a handler for the next received signal. - /// - /// This function handles the next received signal and removes it from the - /// queue of received signals. While the function is executed, all custom - /// signal handlers are blocked to prevent race condition. - /// - /// @param signal_handler A pointer to the signal handler function to - /// be used to handle the signal. - void handleNext(SignalHandler signal_handler); - - /// @brief Uninstalls signal handler for a specified signal. - /// - /// @param sig A code of the signal to be removed. - void remove(const int sig); - - /// @brief Registers a handler as the onreceipt signal handler - /// - /// Sets the given handler as the handler to invoke immediately - /// upon receipt of a a registered signal. - /// - /// @note Currently, the on-receipt handler is stored as a static - /// value and hence there may only be one such handler at a time - /// for a given process. - /// - /// @param handler the signal handler to register - static void setOnReceiptHandler(BoolSignalHandler handler); - - /// @brief Unregisters the onreceipt signal handler - static void clearOnReceiptHandler(); - - /// @brief Invokes the onreceipt handler if it exists - /// - /// This static method is used by @c isc::util::SignalSet class to - /// invoke the registered handler (if one) immediately upon receipt of - /// a registered signal. - /// - /// Prior to invoking the handler, it sets signal action for the - /// given signal to SIG_IGN which prevents any repeat signal - /// occurrences from queuing while the handler is executing. Upon - /// completion of the handler, the signal action is restored which - /// re-enables receipt and handling of the signal. - /// - /// @param sig Signal number. - /// @return Boolean false if no on-receipt handler was registered, - /// otherwise it is the value returned by the on-receipt handler. - static bool invokeOnReceiptHandler(int sig); - -private: - - /// @brief Blocks signals in the set. - /// - /// This function blocks the signals in a set to prevent race condition - /// between the signal handler and the new signal coming in. - void block(); - - /// @brief Removes the signal from the set. - /// - /// This function removes only a signal which is owned by this signal set. - /// - /// @param sig Signal to be removed. - /// @throw SignalSetError if the signal being removed is not owned by this - /// signal set. - void erase(const int sig); - - /// @brief Insert a signal to the set. - /// - /// @param sig Signal to be inserted. - /// @throw SignalSetError if a signal being inserted has already been - /// registered in this or other signal set. - void insert(const int sig); - - /// @brief Applies a mask to all signals in the set. - /// - /// This function is used by @c SignalSet::block and @c SignalSet::unblock - /// to apply the SIG_BLOCK and SIG_UNBLOCK mask to signals. - /// - /// @param mask A mask to be applied to all signals. - /// @param sig Optional signal to be masked. If this value is negative all - /// signals are masked. - void maskSignals(const int mask, const int sig = -1) const; - - /// @brief Pops a next signal number from the static collection of signals. - /// - /// The static collection of signals is updated by the internal signal - /// handler being invoked when one of the installed signals is received by - /// the process. This function removes the first element of the collection. - void popNext(); - - /// @brief Unblocks signals in the set. - /// - /// This function unblocks the signals in a set. - /// - /// @param sig Optional signal to be unblocked. If this is negative, all - /// registered signals are unblocked. - void unblock(int sig = -1); - - /// @brief Indicates if receiving signals is blocked. - bool blocked_; - - /// @brief Stores the set of signals registered in this signal set. - std::set<int> local_signals_; - - /// @brief Shared pointer to static set of registered signals - /// Set during construction to ensure static set does not lose scope - /// before we do. - SigIntSetPtr registered_signals_; - - /// @brief Shared pointer to static list of pending signals - /// Set during construction to ensure static list does not lose scope - /// before we do. - SigIntListPtr signal_states_; -}; - -} -} - -#endif // SIGNAL_SET_H - diff --git a/src/lib/util/tests/Makefile.am b/src/lib/util/tests/Makefile.am index f5b3598f07..d8354e967b 100644 --- a/src/lib/util/tests/Makefile.am +++ b/src/lib/util/tests/Makefile.am @@ -17,10 +17,6 @@ CLEANFILES = *.gcno *.gcda # CSV files are created by unit tests for CSVFile class. CLEANFILES += *.csv -DISTCLEANFILES = process_spawn_app.sh - -noinst_SCRIPTS = process_spawn_app.sh - TESTS_ENVIRONMENT = \ $(LIBTOOL) --mode=execute $(VALGRIND_COMMAND) @@ -48,7 +44,6 @@ run_unittests_SOURCES += memory_segment_common_unittest.cc run_unittests_SOURCES += multi_threading_mgr_unittest.cc run_unittests_SOURCES += optional_unittest.cc run_unittests_SOURCES += pid_file_unittest.cc -run_unittests_SOURCES += process_spawn_unittest.cc run_unittests_SOURCES += qid_gen_unittest.cc run_unittests_SOURCES += random_number_generator_unittest.cc run_unittests_SOURCES += staged_value_unittest.cc @@ -58,7 +53,6 @@ run_unittests_SOURCES += thread_pool_unittest.cc run_unittests_SOURCES += time_utilities_unittest.cc run_unittests_SOURCES += range_utilities_unittest.cc run_unittests_SOURCES += readwrite_mutex_unittest.cc -run_unittests_SOURCES += signal_set_unittest.cc run_unittests_SOURCES += stopwatch_unittest.cc run_unittests_SOURCES += unlock_guard_unittests.cc run_unittests_SOURCES += utf8_unittest.cc diff --git a/src/lib/util/tests/signal_set_unittest.cc b/src/lib/util/tests/signal_set_unittest.cc deleted file mode 100644 index 90c25f583d..0000000000 --- a/src/lib/util/tests/signal_set_unittest.cc +++ /dev/null @@ -1,242 +0,0 @@ -// Copyright (C) 2015-2020 Internet Systems Consortium, Inc. ("ISC") -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -#include <config.h> - -#include <util/signal_set.h> -#include <boost/shared_ptr.hpp> -#include <gtest/gtest.h> -#include <signal.h> - -namespace { - -using namespace isc; -using namespace isc::util; -namespace ph = std::placeholders; - -/// @brief Test fixture class for @c isc::util::SignalSet class. -/// -/// This class contains a handler function which records the signal -/// being handled. It allows for checking whether the signal set -/// has invoked the handler for the expected signal. -class SignalSetTest : public ::testing::Test { -public: - - /// @brief Constructor. - /// - /// Resets the signal sets and variables being modified by the - /// signal handler function. - SignalSetTest() - : signal_set_(), - secondary_signal_set_(), - onreceipt_trues_(0) { - handler_calls_ = 0; - signum_ = -1; - } - - /// @brief Destructor. - /// - /// Uninstalls the signals from the signal sets. - ~SignalSetTest() { - if (signal_set_) { - signal_set_->clear(); - } - if (secondary_signal_set_) { - secondary_signal_set_->clear(); - } - } - - /// @brief Deferred processing signal handler used by unit tests. - /// - /// @param signum Signal being handled. - static void testHandler(int signum) { - signum_ = signum; - ++handler_calls_; - } - - /// @brief Immediate processing signal handler used by unit tests. - /// - /// The handler processes only SIGHUP. All others will pass through. - /// - /// @param signum Signal being handled. - /// @return Boolean true if the handler has processed the signal, false - /// otherwise. - bool onReceiptHandler(int signum) { - if (signum == SIGHUP) { - ++onreceipt_trues_; - return (true); - } - - return (false); - } - - /// @brief Number of handler calls so far. - static int handler_calls_; - /// @brief The last signal handled. - static int signum_; - /// @brief Test signal set object. - SignalSetPtr signal_set_; - /// @brief Second signal set object. - SignalSetPtr secondary_signal_set_; - /// @brief Number of true returns from onReceiptHandler so far. - int onreceipt_trues_; -}; - -int SignalSetTest::handler_calls_ = 0; -int SignalSetTest::signum_ = -1; - -/// Check that the signals are recorded by the signal handlers. -TEST_F(SignalSetTest, twoSignals) { - // Register handlers for two signals. - ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP, SIGINT))); - // Send SIGHUP signal to the process. - ASSERT_EQ(0, raise(SIGHUP)); - // The SIGHUP should be the next one in the queue to be handled. - EXPECT_EQ(SIGHUP, signal_set_->getNext()); - // But, no handlers should have been called yet. - EXPECT_EQ(0, handler_calls_); - // Send a different signal. - ASSERT_EQ(0, raise(SIGINT)); - // The SIGHUP hasn't been handled yet so it should still be the first - // one in the queue. - EXPECT_EQ(SIGHUP, signal_set_->getNext()); - // No handlers have been called yet. - EXPECT_EQ(0, handler_calls_); - // Raise another SIGHUP before the first one has been handled. The - // second one should be dropped. - ASSERT_EQ(0, raise(SIGHUP)); - // Execute the first handler (for SIGHUP). - signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - // The handler should have been called once and the signal - // handled should be SIGHUP. - EXPECT_EQ(1, handler_calls_); - EXPECT_EQ(SIGHUP, signum_); - // Next signal to be handled should be SIGINT. - EXPECT_EQ(SIGINT, signal_set_->getNext()); - signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - EXPECT_EQ(2, handler_calls_); - EXPECT_EQ(SIGINT, signum_); - // There should be no more waiting handlers. - EXPECT_EQ(-1, signal_set_->getNext()); - // Make sure that signals can be unregistered. - EXPECT_NO_THROW(signal_set_->remove(SIGHUP)); - EXPECT_NO_THROW(signal_set_->remove(SIGINT)); -} - -/// Check that the signal set can only handle signals owned by it. -TEST_F(SignalSetTest, twoSignalSets) { - // Register handler for SIGHUP in the first signal set. - signal_set_.reset(new SignalSet(SIGHUP)); - // Register handler for SIGINT in the second signal set. - secondary_signal_set_.reset(new SignalSet(SIGINT)); - // Send SIGHUP. - ASSERT_EQ(0, raise(SIGHUP)); - // Send SIGINT. - ASSERT_EQ(0, raise(SIGINT)); - // Although the SIGHUP is the first signal received by the process - // it is not owned by the secondary signal set. The first signal - // to be handled by the secondary signal set is SIGINT. - EXPECT_EQ(SIGINT, secondary_signal_set_->getNext()); - // The signal set owns SIGHUP so it should be the next to handle. - EXPECT_EQ(SIGHUP, signal_set_->getNext()); - // Handle next signal owned by the secondary signal set. - secondary_signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - EXPECT_EQ(1, handler_calls_); - EXPECT_EQ(SIGINT, signum_); - // No more signals to be handled for this signal set. - EXPECT_EQ(-1, secondary_signal_set_->getNext()); - // Handle next signal owned by the signal set. - signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - EXPECT_EQ(2, handler_calls_); - EXPECT_EQ(SIGHUP, signum_); - // No more signals to be handled by this signal set. - EXPECT_EQ(-1, signal_set_->getNext()); -} - -// Check that each signal set removes only the signals that it has been used -// to register. -TEST_F(SignalSetTest, remove) { - // Register handlers for SIGHUP using one signal set. - ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP))); - // Define another signal set and register a different signal. - ASSERT_NO_THROW(secondary_signal_set_.reset(new SignalSet(SIGINT))); - // The SIGHUP has been already registered with the other signal - // set, so it should not be possible to register it again. - ASSERT_THROW(secondary_signal_set_->add(SIGHUP), SignalSetError); - // It shouldn't be possible to remove the signal registered in a different - // signal set. - ASSERT_THROW(secondary_signal_set_->remove(SIGHUP), SignalSetError); - // Remove all signals from the first signal set. The signals registered - // with the other signal signal set should be preserved. - ASSERT_NO_THROW(signal_set_->clear()); - // Check indirectly that the SIGINT is still registered. An attempt to - // register registered signal should result in failure. - EXPECT_THROW(secondary_signal_set_->add(SIGINT), SignalSetError); - // But, we should be able to register SIGHUP. - EXPECT_NO_THROW(secondary_signal_set_->add(SIGHUP)); -} - -/// Check that it is not possible to duplicate signals. -TEST_F(SignalSetTest, duplicates) { - ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP))); - // It shouldn't be possible to register the same signal. - EXPECT_THROW(signal_set_->add(SIGHUP), SignalSetError); - // But ok to register a different one. - EXPECT_NO_THROW(signal_set_->add(SIGTERM)); - // Now, let's define other signal set. - SignalSetPtr other; - // SIGINT hasn't been registered in the first signal set - // so it should be fine to register. - ASSERT_NO_THROW(other.reset(new SignalSet(SIGINT))); - // SIGHUP has been already registered in the first signal set so - // an attempt to register it again should result in a failure. - EXPECT_THROW(other->add(SIGHUP), SignalSetError); -} - -/// Check that on-receipt processing works. -TEST_F(SignalSetTest, onReceiptTests) { - // Install an on-receipt handler. - SignalSet::setOnReceiptHandler(std::bind(&SignalSetTest::onReceiptHandler, - this, ph::_1)); - // Create a SignalSet for SIGHUP and SIGUSR1. - ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP, SIGUSR1))); - - // Generate SIGHUP, which the on-receipt handler should process. - // Verify that the on-receipt handler processed the signal and that - // no signals are pending. - ASSERT_EQ(0, raise(SIGHUP)); - EXPECT_EQ(1, onreceipt_trues_); - EXPECT_EQ(-1, signal_set_->getNext()); - - // Generate SIGHUP, which the on-receipt handler should NOT process. - // Verify the on-receipt handler did not the signal and that SIGUSR1 - // is pending. - ASSERT_EQ(0, raise(SIGUSR1)); - EXPECT_EQ(1, onreceipt_trues_); - EXPECT_EQ(SIGUSR1, signal_set_->getNext()); - - // Verify we can process SIGUSR1 with the deferred handler. - signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - EXPECT_EQ(1, handler_calls_); - EXPECT_EQ(SIGUSR1, signum_); - - // Unregister the on-receipt handler. - SignalSet::clearOnReceiptHandler(); - - // Generate SIGHUP. - // Verify that the on-receipt handler did not process the signal, and - // that SIGHUP is pending. - ASSERT_EQ(0, raise(SIGHUP)); - EXPECT_EQ(1, onreceipt_trues_); - EXPECT_EQ(SIGHUP, signal_set_->getNext()); - - // Verify we can process it with deferred handler. - signal_set_->handleNext(std::bind(&SignalSetTest::testHandler, ph::_1)); - EXPECT_EQ(2, handler_calls_); - EXPECT_EQ(SIGHUP, signum_); -} - -} // end of anonymous namespace |