From: Razvan Becheriu Date: Fri, 19 Apr 2024 14:53:32 +0000 (+0300) Subject: [#3315] addressed review comments X-Git-Tag: Kea-2.5.8~51 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc104b47a89d3cdf0dab207fc58b32c0f6eb9cee;p=thirdparty%2Fkea.git [#3315] addressed review comments --- diff --git a/src/bin/agent/ca_process.cc b/src/bin/agent/ca_process.cc index 536c1fe29f..2bfa6a77b6 100644 --- a/src/bin/agent/ca_process.cc +++ b/src/bin/agent/ca_process.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -89,7 +90,7 @@ CtrlAgentProcess::run() { size_t CtrlAgentProcess::runIO() { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); size_t cnt = getIOService()->poll(); if (!cnt) { cnt = getIOService()->runOne(); @@ -196,7 +197,7 @@ CtrlAgentProcess::configure(isc::data::ConstElementPtr config_set, /// Let postponed hook initializations to run. try { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); } catch (const std::exception& ex) { std::ostringstream err; err << "Error initializing hooks: " diff --git a/src/bin/agent/simple_parser.cc b/src/bin/agent/simple_parser.cc index 6c6eb5d066..7a6a76372c 100644 --- a/src/bin/agent/simple_parser.cc +++ b/src/bin/agent/simple_parser.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -14,6 +15,7 @@ #include using namespace isc::data; +using namespace isc::asiolink; namespace isc { namespace agent { @@ -182,6 +184,7 @@ AgentSimpleParser::parse(const CtrlAgentCfgContextPtr& ctx, // change causes problems when trying to roll back. HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); + IOServiceMgr::instance().clearIOServices(); libraries.loadLibraries(false); } } diff --git a/src/bin/d2/d2_hooks.dox b/src/bin/d2/d2_hooks.dox index 55410809b7..a01525b5d4 100644 --- a/src/bin/d2/d2_hooks.dox +++ b/src/bin/d2/d2_hooks.dox @@ -57,12 +57,15 @@ to the end of this list. its (re)configuration. The server provides received and parsed configuration structures to the hook library. If the library uses any IO operations, it should create a local IOService - object and register it to the main IOService which is also provided. This way - the local IOService is used by the server to run asynchronous operations. The - hooks library can use the local IOService object to schedule asynchronous - tasks which are triggered by the D2 server's main loop. The hook library - should hold the provided pointer until the library is unloaded at which stage - it must unregister the local IOService. + object and register it to the IOServiceMgr. This way the local IOService is + used by the server to run asynchronous operations. The hooks library can use + the local IOService object to schedule asynchronous tasks which are triggered + by the D2 server's main loop. The hook library can use the local IOService + until the library is unloaded at which stage it must unregister it. + The "io_context" parameter gives access to the main IOService, but it's use + has been deprecated in favor of a local IOService to avoid issues when + unloading the library. The parameter has been deprecated and will be removed + in future versions. The D2CfgContext object provides access to the D2 running configuration. - Next step status: If any callout sets the status to DROP, the server diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index c77430cd82..1e741697e9 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -17,6 +18,7 @@ #include #include +using namespace isc::asiolink; using namespace isc::config; using namespace isc::hooks; using namespace isc::process; @@ -129,7 +131,7 @@ D2Process::run() { size_t D2Process::runIO() { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); // We want to block until at least one handler is called. We'll use // boost::asio::io_service directly for two reasons. First off // asiolink::IOService::runOne is a void and boost::asio::io_service::stopped @@ -303,7 +305,7 @@ D2Process::configure(isc::data::ConstElementPtr config_set, bool check_only) { /// Let postponed hook initializations to run. try { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); } catch (const std::exception& ex) { std::ostringstream err; err << "Error initializing hooks: " diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index a3cc72087d..3145be6a0a 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -241,7 +242,7 @@ ControlledDhcpv4Srv::commandLibReloadHandler(const string&, ConstElementPtr) { HookLibsCollection loaded = HooksManager::getLibraryInfo(); HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); - getIOService()->clearExternalIOServices(); + IOServiceMgr::instance().clearIOServices(); bool multi_threading_enabled = true; uint32_t thread_count = 0; uint32_t queue_size = 0; @@ -453,7 +454,7 @@ ControlledDhcpv4Srv::commandConfigSetHandler(const string&, /// Let postponed hook initializations to run. try { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); } catch (const std::exception& ex) { std::ostringstream err; err << "Error initializing hooks: " diff --git a/src/bin/dhcp4/dhcp4_hooks.dox b/src/bin/dhcp4/dhcp4_hooks.dox index 177e5600ba..cb8c699a20 100644 --- a/src/bin/dhcp4/dhcp4_hooks.dox +++ b/src/bin/dhcp4/dhcp4_hooks.dox @@ -57,12 +57,15 @@ to the end of this list. its (re)configuration. The server provides received and parsed configuration structures to the hook library. If the library uses any IO operations, it should create a local IOService - object and register it to the main IOService which is also provided. This way - the local IOService is used by the server to run asynchronous operations. The - hooks library can use the local IOService object to schedule asynchronous - tasks which are triggered by the DHCP server's main loop. The hook library - should hold the provided pointer until the library is unloaded at which stage - it must unregister the local IOService. + object and register it to the IOServiceMgr. This way the local IOService is + used by the server to run asynchronous operations. The hooks library can use + the local IOService object to schedule asynchronous tasks which are triggered + by the DHCP server's main loop. The hook library can use the local IOService + until the library is unloaded at which stage it must unregister it. + The "io_context" parameter gives access to the main IOService, but it's use + has been deprecated in favor of a local IOService to avoid issues when + unloading the library. The parameter has been deprecated and will be removed + in future versions. The NetworkState object provides access to the DHCP service state of the server and allows for enabling and disabling the DHCP service from the hooks libraries. diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index dd242cd0a5..14ab46a1df 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -706,7 +707,7 @@ Dhcpv4Srv::~Dhcpv4Srv() { } LOG_ERROR(dhcp4_logger, DHCP4_SRV_UNLOAD_LIBRARIES_ERROR).arg(msg); } - getIOService()->clearExternalIOServices(); + IOServiceMgr::instance().clearIOServices(); io_service_->stop(); io_service_->restart(); try { @@ -1133,7 +1134,7 @@ Dhcpv4Srv::run() { #endif // ENABLE_AFL try { runOne(); - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); getIOService()->poll(); } catch (const std::exception& e) { // General catch-all exception that are not caught by more specific diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index 0cfa5de2b7..9ab0c886c5 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -879,6 +880,7 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, // change causes problems when trying to roll back. HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); + IOServiceMgr::instance().clearIOServices(); const HooksConfig& libraries = CfgMgr::instance().getStagingCfg()->getHooksConfig(); bool multi_threading_enabled = true; @@ -934,6 +936,16 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, if (notify_libraries) { return (notify_libraries); } + + /// Let postponed hook initializations to run. + try { + IOServiceMgr::instance().pollIOServices(); + } catch (const std::exception& ex) { + std::ostringstream err; + err << "Error initializing hooks: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } } return (answer); diff --git a/src/bin/dhcp4/tests/callout_library_4.cc b/src/bin/dhcp4/tests/callout_library_4.cc index e9b55d85ad..0166e5f0ee 100644 --- a/src/bin/dhcp4/tests/callout_library_4.cc +++ b/src/bin/dhcp4/tests/callout_library_4.cc @@ -15,6 +15,7 @@ static const int LIBRARY_NUMBER = 4; #include #include +#include #include #include @@ -32,7 +33,6 @@ void start_service(void) { }; IOServicePtr io_service; -IOServicePtr main_io_service; } // end anonymous @@ -57,9 +57,7 @@ do_load_impl(LibraryHandle& handle) { int do_unload_impl() { - if (main_io_service) { - main_io_service->unregisterExternalIOService(io_service); - } + IOServiceMgr::instance().unregisterIOService(io_service); return (0); } @@ -92,14 +90,10 @@ dhcp4_srv_configured(CalloutHandle& handle) { // Get the IO context to post start_service on it. std::string error(""); try { - handle.getArgument("io_context", main_io_service); - if (!main_io_service) { - error = "null io_context"; - } - main_io_service->registerExternalIOService(io_service); + IOServiceMgr::instance().registerIOService(io_service); io_service->post(start_service); } catch (const std::exception& ex) { - error = "no io_context in arguments"; + error = "unknown error"; } if (!error.empty()) { handle.setArgument("error", error); diff --git a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in index 2ab573b2a5..8ea55c1767 100644 --- a/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in +++ b/src/bin/dhcp4/tests/dhcp4_process_tests.sh.in @@ -19,7 +19,9 @@ LEASE_FILE="@abs_top_builddir@/src/bin/dhcp4/tests/test_leases.csv" # Path to the Kea LFC application export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" # Path to test hooks library -HOOK_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" +HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco3.so" +# Path to test hooks library +HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp4/tests/.libs/libco4.so" # Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp4\": @@ -202,7 +204,7 @@ INVALID_CONFIG_HOOKS_LOAD="{ }, \"hooks-libraries\": [ { - \"library\": \"$HOOK_PATH\", + \"library\": \"$HOOK_FAIL_LOAD_PATH\", \"parameters\": { \"mode\": \"fail-on-load\" } @@ -223,14 +225,66 @@ INVALID_CONFIG_HOOKS_LOAD="{ # Invalid configuration (hook point returns error) to check that performing # extra configuration checks detects the error. -INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ +INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_LOAD="{ \"Dhcp4\": { \"interfaces-config\": { \"interfaces\": [ ] }, \"multi-threading\": { - \"enable-multi-threading\": false + \"enable-multi-threading\": false + }, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet4\": [ + { + \"id\": 1, + \"subnet\": \"10.0.0.0/8\", + \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_FAIL_LOAD_PATH\", + \"parameters\": { + \"mode\": \"fail-without-error\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp4\", + \"output-options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" + +# Invalid configuration (poll after load throws exception) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_POLL="{ + \"Dhcp4\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false }, \"valid-lifetime\": 4000, \"renew-timer\": 1000, @@ -254,7 +308,7 @@ INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ }, \"hooks-libraries\": [ { - \"library\": \"$HOOK_PATH\", + \"library\": \"$HOOK_FAIL_POLL_PATH\", \"parameters\": { \"mode\": \"fail-without-error\" } @@ -582,5 +636,6 @@ syntax_check_test "dhcpv4.syntax_check_success" "${CONFIG}" 0 -t syntax_check_test "dhcpv4.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 -t syntax_check_test "dhcpv4.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 -t syntax_check_test "dhcpv4.syntax_check_hooks_load_fail" "${INVALID_CONFIG_HOOKS_LOAD}" 1 -T -syntax_check_test "dhcpv4.syntax_check_hooks_callout_fail" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL}" 1 -T +syntax_check_test "dhcpv4.syntax_check_hooks_callout_fail_on_load" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_LOAD}" 1 -T +syntax_check_test "dhcpv4.syntax_check_hooks_callout_fail_on_poll" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_POLL}" 1 -T password_redact_test "dhcpv4.password_redact_test" "$(kea_dhcp_config 4)" 0 diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index bfea49d256..1e4af158d1 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -244,7 +245,7 @@ ControlledDhcpv6Srv::commandLibReloadHandler(const string&, ConstElementPtr) { HookLibsCollection loaded = HooksManager::getLibraryInfo(); HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); - getIOService()->clearExternalIOServices(); + IOServiceMgr::instance().clearIOServices(); bool multi_threading_enabled = true; uint32_t thread_count = 0; uint32_t queue_size = 0; @@ -455,7 +456,7 @@ ControlledDhcpv6Srv::commandConfigSetHandler(const string&, /// Let postponed hook initializations to run. try { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); } catch (const std::exception& ex) { std::ostringstream err; err << "Error initializing hooks: " diff --git a/src/bin/dhcp6/dhcp6_hooks.dox b/src/bin/dhcp6/dhcp6_hooks.dox index 7c7e5dfa53..d2b3087950 100644 --- a/src/bin/dhcp6/dhcp6_hooks.dox +++ b/src/bin/dhcp6/dhcp6_hooks.dox @@ -57,12 +57,15 @@ to the end of this list. its (re)configuration. The server provides received and parsed configuration structures to the hook library. If the library uses any IO operations, it should create a local IOService - object and register it to the main IOService which is also provided. This way - the local IOService is used by the server to run asynchronous operations. The - hooks library can use the local IOService object to schedule asynchronous - tasks which are triggered by the DHCP server's main loop. The hook library - should hold the provided pointer until the library is unloaded at which stage - it must unregister the local IOService. + object and register it to the IOServiceMgr. This way the local IOService is + used by the server to run asynchronous operations. The hooks library can use + the local IOService object to schedule asynchronous tasks which are triggered + by the DHCP server's main loop. The hook library can use the local IOService + until the library is unloaded at which stage it must unregister it. + The "io_context" parameter gives access to the main IOService, but it's use + has been deprecated in favor of a local IOService to avoid issues when + unloading the library. The parameter has been deprecated and will be removed + in future versions. The NetworkState object provides access to the DHCP service state of the server and allows for enabling and disabling the DHCP service from the hooks libraries. diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 0f59bc4481..205bf5d23d 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -302,7 +303,7 @@ Dhcpv6Srv::~Dhcpv6Srv() { } LOG_ERROR(dhcp6_logger, DHCP6_SRV_UNLOAD_LIBRARIES_ERROR).arg(msg); } - getIOService()->clearExternalIOServices(); + IOServiceMgr::instance().clearIOServices(); io_service_->stop(); io_service_->restart(); try { @@ -614,7 +615,7 @@ Dhcpv6Srv::run() { #endif // ENABLE_AFL try { runOne(); - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); getIOService()->poll(); } catch (const std::exception& e) { // General catch-all standard exceptions that are not caught by more diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index 207dde7b0b..8de3907438 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -1012,6 +1013,7 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, // change causes problems when trying to roll back. HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); + IOServiceMgr::instance().clearIOServices(); const HooksConfig& libraries = CfgMgr::instance().getStagingCfg()->getHooksConfig(); bool multi_threading_enabled = true; @@ -1067,6 +1069,16 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, if (notify_libraries) { return (notify_libraries); } + + /// Let postponed hook initializations to run. + try { + IOServiceMgr::instance().pollIOServices(); + } catch (const std::exception& ex) { + std::ostringstream err; + err << "Error initializing hooks: " + << ex.what(); + return (isc::config::createAnswer(CONTROL_RESULT_ERROR, err.str())); + } } return (answer); diff --git a/src/bin/dhcp6/tests/callout_library_4.cc b/src/bin/dhcp6/tests/callout_library_4.cc index 0739268fc1..537315c4c5 100644 --- a/src/bin/dhcp6/tests/callout_library_4.cc +++ b/src/bin/dhcp6/tests/callout_library_4.cc @@ -15,6 +15,7 @@ static const int LIBRARY_NUMBER = 4; #include #include +#include #include #include @@ -32,7 +33,6 @@ void start_service(void) { }; IOServicePtr io_service; -IOServicePtr main_io_service; } // end anonymous @@ -57,9 +57,7 @@ do_load_impl(LibraryHandle& handle) { int do_unload_impl() { - if (main_io_service) { - main_io_service->unregisterExternalIOService(io_service); - } + IOServiceMgr::instance().unregisterIOService(io_service); return (0); } @@ -92,14 +90,10 @@ dhcp6_srv_configured(CalloutHandle& handle) { // Get the IO context to post start_service on it. std::string error(""); try { - handle.getArgument("io_context", main_io_service); - if (!main_io_service) { - error = "null io_context"; - } - main_io_service->registerExternalIOService(io_service); + IOServiceMgr::instance().registerIOService(io_service); io_service->post(start_service); } catch (const std::exception& ex) { - error = "no io_context in arguments"; + error = "unknown error"; } if (!error.empty()) { handle.setArgument("error", error); diff --git a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in index 3bcab77cb3..ad747214b9 100644 --- a/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in +++ b/src/bin/dhcp6/tests/dhcp6_process_tests.sh.in @@ -19,7 +19,9 @@ LEASE_FILE="@abs_top_builddir@/src/bin/dhcp6/tests/test_leases.csv" # Path to the Kea LFC application export KEA_LFC_EXECUTABLE="@abs_top_builddir@/src/bin/lfc/kea-lfc" # Path to test hooks library -HOOK_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" +HOOK_FAIL_LOAD_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" +# Path to test hooks library +HOOK_FAIL_POLL_PATH="@abs_top_builddir@/src/bin/dhcp6/tests/.libs/libco3.so" # Kea configuration to be stored in the configuration file. CONFIG="{ \"Dhcp6\": @@ -214,7 +216,7 @@ INVALID_CONFIG_HOOKS_LOAD="{ }, \"hooks-libraries\": [ { - \"library\": \"$HOOK_PATH\", + \"library\": \"$HOOK_FAIL_LOAD_PATH\", \"parameters\": { \"mode\": \"fail-on-load\" } @@ -235,18 +237,75 @@ INVALID_CONFIG_HOOKS_LOAD="{ # Invalid configuration (hook point returns error) to check that performing # extra configuration checks detects the error. -INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ +INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_LOAD="{ \"Dhcp6\": { \"interfaces-config\": { - \"interfaces\": [ ] + \"interfaces\": [ ] }, \"multi-threading\": { - \"enable-multi-threading\": false + \"enable-multi-threading\": false }, \"server-id\": { - \"type\": \"LLT\", - \"persist\": false + \"type\": \"LLT\", + \"persist\": false + }, + \"preferred-lifetime\": 3000, + \"valid-lifetime\": 4000, + \"renew-timer\": 1000, + \"rebind-timer\": 2000, + \"lease-database\": + { + \"type\": \"memfile\", + \"name\": \"$LEASE_FILE\", + \"persist\": false, + \"lfc-interval\": 0 + }, + \"subnet6\": [ + { + \"subnet\": \"2001:db8:1::/64\", + \"id\": 1, + \"pools\": [ { \"pool\": \"2001:db8:1::10-2001:db8:1::100\" } ] + } ], + \"dhcp-ddns\": { + \"enable-updates\": true, + \"qualifying-suffix\": \"\" + }, + \"hooks-libraries\": [ + { + \"library\": \"$HOOK_FAIL_LOAD_PATH\", + \"parameters\": { + \"mode\": \"fail-without-error\" + } + } ], + \"loggers\": [ + { + \"name\": \"kea-dhcp6\", + \"output-options\": [ + { + \"output\": \"$LOG_FILE\" + } + ], + \"severity\": \"INFO\" + } + ] + } +}" + +# Invalid configuration (poll after load throws exception) to check that performing +# extra configuration checks detects the error. +INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_POLL="{ + \"Dhcp6\": + { + \"interfaces-config\": { + \"interfaces\": [ ] + }, + \"multi-threading\": { + \"enable-multi-threading\": false + }, + \"server-id\": { + \"type\": \"LLT\", + \"persist\": false }, \"preferred-lifetime\": 3000, \"valid-lifetime\": 4000, @@ -271,7 +330,7 @@ INVALID_CONFIG_HOOKS_CALLOUT_FAIL="{ }, \"hooks-libraries\": [ { - \"library\": \"$HOOK_PATH\", + \"library\": \"$HOOK_FAIL_POLL_PATH\", \"parameters\": { \"mode\": \"fail-without-error\" } @@ -602,5 +661,6 @@ syntax_check_test "dhcpv6.syntax_check_success" "${CONFIG}" 0 -t syntax_check_test "dhcpv6.syntax_check_bad_syntax" "${CONFIG_BAD_SYNTAX}" 1 -t syntax_check_test "dhcpv6.syntax_check_bad_values" "${CONFIG_BAD_VALUES}" 1 -t syntax_check_test "dhcpv6.syntax_check_hooks_load_fail" "${INVALID_CONFIG_HOOKS_LOAD}" 1 -T -syntax_check_test "dhcpv6.syntax_check_hooks_callout_fail" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL}" 1 -T +syntax_check_test "dhcpv6.syntax_check_hooks_callout_fail_on_load" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_LOAD}" 1 -T +syntax_check_test "dhcpv6.syntax_check_hooks_callout_fail_on_poll" "${INVALID_CONFIG_HOOKS_CALLOUT_FAIL_ON_POLL}" 1 -T password_redact_test "dhcpv6.password_redact_test" "$(kea_dhcp_config 6)" 0 diff --git a/src/bin/netconf/netconf_process.cc b/src/bin/netconf/netconf_process.cc index 0b3dcb0a36..2e86a9bee0 100644 --- a/src/bin/netconf/netconf_process.cc +++ b/src/bin/netconf/netconf_process.cc @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -65,7 +66,7 @@ NetconfProcess::run() { size_t NetconfProcess::runIO() { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); size_t cnt = getIOService()->poll(); if (!cnt) { cnt = getIOService()->runOne(); @@ -89,7 +90,7 @@ NetconfProcess::configure(isc::data::ConstElementPtr config_set, /// Let postponed hook initializations to run. try { - getIOService()->pollExternalIOServices(); + IOServiceMgr::instance().pollIOServices(); } catch (const std::exception& ex) { std::ostringstream err; err << "Error initializing hooks: " diff --git a/src/bin/netconf/simple_parser.cc b/src/bin/netconf/simple_parser.cc index fa90fb081e..d658d9f248 100644 --- a/src/bin/netconf/simple_parser.cc +++ b/src/bin/netconf/simple_parser.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -14,6 +15,7 @@ #include using namespace isc::data; +using namespace isc::asiolink; namespace isc { namespace netconf { @@ -189,6 +191,7 @@ NetconfSimpleParser::parse(const NetconfConfigPtr& ctx, // change causes problems when trying to roll back. HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); + IOServiceMgr::instance().clearIOServices(); libraries.loadLibraries(false); } } diff --git a/src/hooks/dhcp/high_availability/ha.dox b/src/hooks/dhcp/high_availability/ha.dox index cb16c0b4a7..2ac714f987 100644 --- a/src/hooks/dhcp/high_availability/ha.dox +++ b/src/hooks/dhcp/high_availability/ha.dox @@ -131,20 +131,25 @@ loop), but each loop pass contains a call to: @code getIOService()->poll(); +IOServiceMgr::instance().pollIOServices(); @endcode which executes callbacks for completed asynchronous operations, such as timers, asynchronous sends and receives. The instance of the IOService -is owned by the DHCP servers, but hooks libraries must have access to it -and must use this instance to schedule asynchronous tasks. This is why -the new hook points "dhcp4_srv_configured" and "dhcp6_srv_configured" -have been introduced. These hook points are used by the DHCPv4 and the -DHCPv6 servers respectively, to pass the instance of the IOService -(via "io_context" argument) to the hooks libraries which require to -schedule asynchronous tasks. -The hook's IOService object must be registered on the server's main IOService by -calling registerExternalIOService and must unregister it on "unload" hook point -by calling unregisterExternalIOService. +is owned by the DHCP servers, but hooks libraries must create their own +IOService access to schedule asynchronous tasks. +The hook's IOService object must be registered on the IOServiceMgr by +calling registerIOService and must unregister it on "unload" hook point +by calling unregisterIOService. + + +The hook points "dhcp4_srv_configured" and "dhcp6_srv_configured" have been +introduced to give access to the server configuration if needed. +These hook points are used by the DHCPv4 and the DHCPv6 servers respectively, to +pass the server configuration to the hooks libraries which require it. +The "io_context" parameter gives access to the main IOService, but it's use +has been deprecated in favor of a local IOService to avoid issues when +unloading the library. The parameter will be removed in future versions. It is also worth to note that the blocking reception of the DHCP packets may cause up to 1 second delays in the asynchronous operations. This is @@ -152,25 +157,27 @@ due to the structure of the main server loop: @code bool -Dhcpv4Srv::run() { +Dhcpv[4|6]Srv::run() { +[...] while (!shutdown_) { +[...] try { runOne(); + IOServiceMgr::instance().pollIOServices(); getIOService()->poll(); } catch (const std::exception& e) { // General catch-all exception that are not caught by more specific // catches. This one is for exceptions derived from std::exception. - LOG_ERROR(packet4_logger, DHCP4_PACKET_PROCESS_STD_EXCEPTION) - .arg(e.what()); + [...] } catch (...) { // General catch-all exception that are not caught by more specific // catches. This one is for other exceptions, not derived from // std::exception. - LOG_ERROR(packet4_logger, DHCP4_PACKET_PROCESS_EXCEPTION); + [...] } } - return (true); + [...] } @endcode diff --git a/src/hooks/dhcp/high_availability/ha_callouts.cc b/src/hooks/dhcp/high_availability/ha_callouts.cc index 200efd0f6b..014080f8e8 100644 --- a/src/hooks/dhcp/high_availability/ha_callouts.cc +++ b/src/hooks/dhcp/high_availability/ha_callouts.cc @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -31,6 +32,7 @@ HAImplPtr impl; } // end of namespace isc::ha } // end of namespace isc +using namespace isc::asiolink; using namespace isc::config; using namespace isc::data; using namespace isc::dhcp; @@ -46,18 +48,10 @@ extern "C" { /// @param handle callout handle. int dhcp4_srv_configured(CalloutHandle& handle) { try { - handle.getArgument("io_context", impl->getMainIOService()); - if (!impl->getMainIOService()) { - // Should not happen! - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - const string error("Error: io_context is null"); - handle.setArgument("error", error); - return (1); - } isc::dhcp::NetworkStatePtr network_state; handle.getArgument("network_state", network_state); impl->startServices(network_state, HAServerType::DHCPv4); - impl->getMainIOService()->registerExternalIOService(impl->getIOService()); + IOServiceMgr::instance().registerIOService(impl->getIOService()); } catch (const std::exception& ex) { LOG_ERROR(ha_logger, HA_DHCP4_START_SERVICE_FAILED) @@ -164,18 +158,10 @@ int lease4_server_decline(CalloutHandle& handle) { /// @param handle callout handle. int dhcp6_srv_configured(CalloutHandle& handle) { try { - handle.getArgument("io_context", impl->getMainIOService()); - if (!impl->getMainIOService()) { - // Should not happen! - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - const string error("Error: io_context is null"); - handle.setArgument("error", error); - return (1); - } isc::dhcp::NetworkStatePtr network_state; handle.getArgument("network_state", network_state); impl->startServices(network_state, HAServerType::DHCPv6); - impl->getMainIOService()->registerExternalIOService(impl->getIOService()); + IOServiceMgr::instance().registerIOService(impl->getIOService()); } catch (const std::exception& ex) { LOG_ERROR(ha_logger, HA_DHCP6_START_SERVICE_FAILED) @@ -443,9 +429,7 @@ int load(LibraryHandle& handle) { /// @return 0 if deregistration was successful, 1 otherwise int unload() { if (impl) { - if (impl->getMainIOService()) { - impl->getMainIOService()->unregisterExternalIOService(impl->getIOService()); - } + IOServiceMgr::instance().unregisterIOService(impl->getIOService()); impl->getIOService()->stop(); impl->getIOService()->restart(); try { diff --git a/src/hooks/dhcp/high_availability/ha_impl.h b/src/hooks/dhcp/high_availability/ha_impl.h index 041ade8c82..d7920cf737 100644 --- a/src/hooks/dhcp/high_availability/ha_impl.h +++ b/src/hooks/dhcp/high_availability/ha_impl.h @@ -224,7 +224,7 @@ public: /// @brief Get the hook I/O service. /// /// @return the hook I/O service. - isc::asiolink::IOServicePtr& getIOService() { + isc::asiolink::IOServicePtr getIOService() { return (io_service_); } @@ -235,28 +235,11 @@ public: io_service_ = io_service; } - /// @brief Get the main I/O service. - /// - /// @return the main I/O service. - isc::asiolink::IOServicePtr& getMainIOService() { - return (main_io_service_); - } - - /// @brief Set the main I/O service. - /// - /// @param io_service the main I/O service. - void setMainIOService(isc::asiolink::IOServicePtr io_service) { - main_io_service_ = io_service; - } - protected: /// @brief The hook I/O service. isc::asiolink::IOServicePtr io_service_; - /// @brief The main I/O service. - isc::asiolink::IOServicePtr main_io_service_; - /// @brief Holds parsed configuration. HAConfigMapperPtr config_; diff --git a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc index 41c5a57f60..b1b148cc80 100644 --- a/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc +++ b/src/hooks/dhcp/high_availability/libloadtests/close_unittests.cc @@ -380,7 +380,6 @@ TEST_F(CloseHATest, close4) { EXPECT_TRUE(HooksManager::calloutsPresent(testHooks.hook_index_dhcp4_srv_configured_)); { CalloutHandlePtr handle = HooksManager::createCalloutHandle(); - handle->setArgument("io_context", io_service); handle->setArgument("network_state", network_state); HooksManager::callCallouts(testHooks.hook_index_dhcp4_srv_configured_, *handle); @@ -515,7 +514,6 @@ TEST_F(CloseHATest, close4Backup) { EXPECT_TRUE(HooksManager::calloutsPresent(testHooks.hook_index_dhcp4_srv_configured_)); { CalloutHandlePtr handle = HooksManager::createCalloutHandle(); - handle->setArgument("io_context", io_service); handle->setArgument("network_state", network_state); HooksManager::callCallouts(testHooks.hook_index_dhcp4_srv_configured_, *handle); @@ -619,7 +617,6 @@ TEST_F(CloseHATest, close6) { EXPECT_TRUE(HooksManager::calloutsPresent(testHooks.hook_index_dhcp6_srv_configured_)); { CalloutHandlePtr handle = HooksManager::createCalloutHandle(); - handle->setArgument("io_context", io_service); handle->setArgument("network_state", network_state); HooksManager::callCallouts(testHooks.hook_index_dhcp6_srv_configured_, *handle); @@ -754,7 +751,6 @@ TEST_F(CloseHATest, close6Backup) { EXPECT_TRUE(HooksManager::calloutsPresent(testHooks.hook_index_dhcp6_srv_configured_)); { CalloutHandlePtr handle = HooksManager::createCalloutHandle(); - handle->setArgument("io_context", io_service); handle->setArgument("network_state", network_state); HooksManager::callCallouts(testHooks.hook_index_dhcp6_srv_configured_, *handle); diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc index 81750ed82b..f9365d5413 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_test.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc @@ -71,7 +71,6 @@ void HATest::startHAService() { if (HooksManager::calloutsPresent(Hooks.hooks_index_dhcp4_srv_configured_)) { CalloutHandlePtr callout_handle = HooksManager::createCalloutHandle(); - callout_handle->setArgument("io_context", io_service_); callout_handle->setArgument("network_state", network_state_); HooksManager::callCallouts(Hooks.hooks_index_dhcp4_srv_configured_, *callout_handle); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_callouts.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_callouts.cc index 2186dbf312..a3f5e5bab0 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_callouts.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_callouts.cc @@ -10,6 +10,7 @@ #include +#include #include #include #include @@ -68,15 +69,8 @@ int load(LibraryHandle& /* handle */) { /// @param handle callout handle passed to the callout. /// @return 0 on success, 1 otherwise. int dhcp4_srv_configured(CalloutHandle& handle) { - handle.getArgument("io_context", isc::dhcp::MySqlConfigBackendImpl::getMainIOService()); - if (!isc::dhcp::MySqlConfigBackendImpl::getMainIOService()) { - const string error("Error: io_context is null"); - handle.setArgument("error", error); - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - return (1); - } isc::dhcp::MySqlConfigBackendImpl::getIOService().reset(new IOService()); - isc::dhcp::MySqlConfigBackendImpl::getMainIOService()->registerExternalIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); + IOServiceMgr::instance().registerIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); return (0); } @@ -87,15 +81,8 @@ int dhcp4_srv_configured(CalloutHandle& handle) { /// @param handle callout handle passed to the callout. /// @return 0 on success, 1 otherwise. int dhcp6_srv_configured(CalloutHandle& handle) { - handle.getArgument("io_context", isc::dhcp::MySqlConfigBackendImpl::getMainIOService()); - if (!isc::dhcp::MySqlConfigBackendImpl::getMainIOService()) { - const string error("Error: io_context is null"); - handle.setArgument("error", error); - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - return (1); - } isc::dhcp::MySqlConfigBackendImpl::getIOService().reset(new IOService()); - isc::dhcp::MySqlConfigBackendImpl::getMainIOService()->registerExternalIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); + IOServiceMgr::instance().registerIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); return (0); } @@ -107,9 +94,7 @@ int unload() { // Unregister the factories and remove MySQL backends isc::dhcp::MySqlConfigBackendDHCPv4::unregisterBackendType(); isc::dhcp::MySqlConfigBackendDHCPv6::unregisterBackendType(); - if (isc::dhcp::MySqlConfigBackendImpl::getMainIOService()) { - isc::dhcp::MySqlConfigBackendImpl::getMainIOService()->unregisterExternalIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); - } + IOServiceMgr::instance().unregisterIOService(isc::dhcp::MySqlConfigBackendImpl::getIOService()); if (isc::dhcp::MySqlConfigBackendImpl::getIOService()) { isc::dhcp::MySqlConfigBackendImpl::getIOService()->stop(); isc::dhcp::MySqlConfigBackendImpl::getIOService()->restart(); diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc index 390bf9cd2f..4e1475b363 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.cc @@ -30,7 +30,6 @@ namespace isc { namespace dhcp { isc::asiolink::IOServicePtr MySqlConfigBackendImpl::io_service_; -isc::asiolink::IOServicePtr MySqlConfigBackendImpl::main_io_service_; MySqlConfigBackendImpl:: ScopedAuditRevision::ScopedAuditRevision(MySqlConfigBackendImpl* impl, diff --git a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h index 9931c8a551..24de344f5f 100644 --- a/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h +++ b/src/hooks/dhcp/mysql_cb/mysql_cb_impl.h @@ -839,7 +839,7 @@ public: /// @brief Get the hook I/O service. /// /// @return the hook I/O service. - static isc::asiolink::IOServicePtr& getIOService() { + static isc::asiolink::IOServicePtr getIOService() { return (io_service_); } @@ -850,20 +850,6 @@ public: io_service_ = io_service; } - /// @brief Get the main I/O service. - /// - /// @return the main I/O service. - static isc::asiolink::IOServicePtr& getMainIOService() { - return (main_io_service_); - } - - /// @brief Set the main I/O service. - /// - /// @param io_service the main I/O service. - static void setMainIOService(isc::asiolink::IOServicePtr io_service) { - main_io_service_ = io_service; - } - /// @brief Represents connection to the MySQL database. db::MySqlConnection conn_; @@ -882,9 +868,6 @@ private: /// @brief The hook I/O service. static isc::asiolink::IOServicePtr io_service_; - - /// @brief The main I/O service. - static isc::asiolink::IOServicePtr main_io_service_; }; } // end of namespace isc::dhcp diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_callouts.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_callouts.cc index f824a0c59f..03a24ac750 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_callouts.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_callouts.cc @@ -10,6 +10,7 @@ #include +#include #include #include #include @@ -68,15 +69,8 @@ int load(LibraryHandle& /* handle */) { /// @param handle callout handle passed to the callout. /// @return 0 on success, 1 otherwise. int dhcp4_srv_configured(CalloutHandle& handle) { - handle.getArgument("io_context", isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()); - if (!isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()) { - const string error("Error: io_context is null"); - handle.setArgument("error", error); - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - return (1); - } isc::dhcp::PgSqlConfigBackendImpl::getIOService().reset(new IOService()); - isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()->registerExternalIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); + IOServiceMgr::instance().registerIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); return (0); } @@ -87,15 +81,8 @@ int dhcp4_srv_configured(CalloutHandle& handle) { /// @param handle callout handle passed to the callout. /// @return 0 on success, 1 otherwise. int dhcp6_srv_configured(CalloutHandle& handle) { - handle.getArgument("io_context", isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()); - if (!isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()) { - const string error("Error: io_context is null"); - handle.setArgument("error", error); - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - return (1); - } isc::dhcp::PgSqlConfigBackendImpl::getIOService().reset(new IOService()); - isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()->registerExternalIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); + IOServiceMgr::instance().registerIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); return (0); } @@ -107,9 +94,7 @@ int unload() { // Unregister the factories and remove PostgreSQL backends isc::dhcp::PgSqlConfigBackendDHCPv4::unregisterBackendType(); isc::dhcp::PgSqlConfigBackendDHCPv6::unregisterBackendType(); - if (isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()) { - isc::dhcp::PgSqlConfigBackendImpl::getMainIOService()->unregisterExternalIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); - } + IOServiceMgr::instance().unregisterIOService(isc::dhcp::PgSqlConfigBackendImpl::getIOService()); if (isc::dhcp::PgSqlConfigBackendImpl::getIOService()) { isc::dhcp::PgSqlConfigBackendImpl::getIOService()->stop(); isc::dhcp::PgSqlConfigBackendImpl::getIOService()->restart(); diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc index 2104e450a1..76a643458f 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.cc @@ -29,7 +29,6 @@ namespace isc { namespace dhcp { isc::asiolink::IOServicePtr PgSqlConfigBackendImpl::io_service_; -isc::asiolink::IOServicePtr PgSqlConfigBackendImpl::main_io_service_; PgSqlTaggedStatement& PgSqlConfigBackendImpl::getStatement(size_t /* index */) const { diff --git a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h index a4d1d93db4..2c848082bb 100644 --- a/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h +++ b/src/hooks/dhcp/pgsql_cb/pgsql_cb_impl.h @@ -863,7 +863,7 @@ public: /// @brief Get the hook I/O service. /// /// @return the hook I/O service. - static isc::asiolink::IOServicePtr& getIOService() { + static isc::asiolink::IOServicePtr getIOService() { return (io_service_); } @@ -874,20 +874,6 @@ public: io_service_ = io_service; } - /// @brief Get the main I/O service. - /// - /// @return the main I/O service. - static isc::asiolink::IOServicePtr& getMainIOService() { - return (main_io_service_); - } - - /// @brief Set the main I/O service. - /// - /// @param io_service the main I/O service. - static void setMainIOService(isc::asiolink::IOServicePtr io_service) { - main_io_service_ = io_service; - } - /// @brief Represents connection to the PostgreSQL database. db::PgSqlConnection conn_; @@ -907,9 +893,6 @@ private: /// @brief The hook I/O service. static isc::asiolink::IOServicePtr io_service_; - /// @brief The main I/O service. - static isc::asiolink::IOServicePtr main_io_service_; - /// @brief Statement index of the SQL statement to use for fetching /// last inserted id in a given table. size_t last_insert_id_index_; diff --git a/src/hooks/dhcp/run_script/run_script.cc b/src/hooks/dhcp/run_script/run_script.cc index 54ddb4139b..f496cfcbe4 100644 --- a/src/hooks/dhcp/run_script/run_script.cc +++ b/src/hooks/dhcp/run_script/run_script.cc @@ -19,7 +19,6 @@ namespace isc { namespace run_script { IOServicePtr RunScriptImpl::io_service_; -IOServicePtr RunScriptImpl::main_io_service_; RunScriptImpl::RunScriptImpl() : io_context_(new IOService()), name_(), sync_(false) { } diff --git a/src/hooks/dhcp/run_script/run_script.h b/src/hooks/dhcp/run_script/run_script.h index dc9f9b8a42..b276de76df 100644 --- a/src/hooks/dhcp/run_script/run_script.h +++ b/src/hooks/dhcp/run_script/run_script.h @@ -245,7 +245,7 @@ public: /// @brief Get the hook I/O service. /// /// @return the hook I/O service. - isc::asiolink::IOServicePtr& getIOContext() { + isc::asiolink::IOServicePtr getIOContext() { return (io_context_); } @@ -259,7 +259,7 @@ public: /// @brief Get the hook I/O service. /// /// @return the hook I/O service. - static isc::asiolink::IOServicePtr& getIOService() { + static isc::asiolink::IOServicePtr getIOService() { return (io_service_); } @@ -270,20 +270,6 @@ public: io_service_ = io_service; } - /// @brief Get the main I/O service. - /// - /// @return the main I/O service. - static isc::asiolink::IOServicePtr& getMainIOService() { - return (main_io_service_); - } - - /// @brief Set the main I/O service. - /// - /// @param io_service the main I/O service. - static void setMainIOService(isc::asiolink::IOServicePtr io_service) { - main_io_service_ = io_service; - } - private: /// @brief The IOService object, used for all ASIO operations. @@ -301,9 +287,6 @@ private: /// @brief The hook I/O service. static isc::asiolink::IOServicePtr io_service_; - - /// @brief The main I/O service. - static isc::asiolink::IOServicePtr main_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 22ee765c9a..2f54de78a4 100644 --- a/src/hooks/dhcp/run_script/run_script_callouts.cc +++ b/src/hooks/dhcp/run_script/run_script_callouts.cc @@ -6,6 +6,7 @@ #include +#include #include #include #include @@ -80,8 +81,8 @@ int load(LibraryHandle& handle) { /// /// @return always 0. int unload() { - if (RunScriptImpl::getMainIOService()) { - RunScriptImpl::getMainIOService()->unregisterExternalIOService(impl->getIOContext()); + if (impl) { + IOServiceMgr::instance().unregisterIOService(impl->getIOContext()); } impl.reset(); if (RunScriptImpl::getIOService()) { @@ -102,16 +103,8 @@ int unload() { /// @param handle callout handle. int dhcp4_srv_configured(CalloutHandle& handle) { try { - handle.getArgument("io_context", RunScriptImpl::getMainIOService()); - if (!RunScriptImpl::getMainIOService()) { - // Should not happen! - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - const string error("Error: io_context is null"); - handle.setArgument("error", error); - return (1); - } RunScriptImpl::setIOService(impl->getIOContext()); - RunScriptImpl::getMainIOService()->registerExternalIOService(impl->getIOContext()); + IOServiceMgr::instance().registerIOService(impl->getIOContext()); } catch (const exception& ex) { LOG_ERROR(run_script_logger, RUN_SCRIPT_LOAD_ERROR) @@ -127,16 +120,8 @@ int dhcp4_srv_configured(CalloutHandle& handle) { /// @param handle callout handle. int dhcp6_srv_configured(CalloutHandle& handle) { try { - handle.getArgument("io_context", RunScriptImpl::getMainIOService()); - if (!RunScriptImpl::getMainIOService()) { - // Should not happen! - handle.setStatus(isc::hooks::CalloutHandle::NEXT_STEP_DROP); - const string error("Error: io_context is null"); - handle.setArgument("error", error); - return (1); - } RunScriptImpl::setIOService(impl->getIOContext()); - RunScriptImpl::getMainIOService()->registerExternalIOService(impl->getIOContext()); + IOServiceMgr::instance().registerIOService(impl->getIOContext()); } catch (const exception& ex) { LOG_ERROR(run_script_logger, RUN_SCRIPT_LOAD_ERROR) diff --git a/src/lib/asiolink/Makefile.am b/src/lib/asiolink/Makefile.am index 213322cb6a..dcf58f2345 100644 --- a/src/lib/asiolink/Makefile.am +++ b/src/lib/asiolink/Makefile.am @@ -29,6 +29,7 @@ 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_mgr.h io_service_mgr.cc libkea_asiolink_la_SOURCES += io_service_signal.cc io_service_signal.h libkea_asiolink_la_SOURCES += io_service_thread_pool.cc io_service_thread_pool.h libkea_asiolink_la_SOURCES += io_socket.h io_socket.cc @@ -82,6 +83,7 @@ libkea_asiolink_include_HEADERS = \ io_endpoint.h \ io_error.h \ io_service.h \ + io_service_mgr.h \ io_service_signal.h \ io_service_thread_pool.h \ io_socket.h \ diff --git a/src/lib/asiolink/io_service.cc b/src/lib/asiolink/io_service.cc index 8c42d6855f..99d458a2bc 100644 --- a/src/lib/asiolink/io_service.cc +++ b/src/lib/asiolink/io_service.cc @@ -40,7 +40,7 @@ public: /// @brief Start the underlying event loop. /// /// This method does not return control to the caller until - /// the @ref stop() method is called via some handler. + /// the @ref stop() or @ref stopWork() method is called via some handler. void run() { io_service_.run(); }; @@ -179,25 +179,5 @@ IOService::post(const std::function& callback) { return (io_impl_->post(callback)); } -void -IOService::registerExternalIOService(IOServicePtr io_service) { - external_io_services_.push_back(io_service); -} - -void -IOService::unregisterExternalIOService(IOServicePtr io_service) { - auto it = std::find(external_io_services_.begin(), external_io_services_.end(), io_service); - if (it != external_io_services_.end()) { - external_io_services_.erase(it); - } -} - -void -IOService::pollExternalIOServices() { - for (auto& io_service : external_io_services_) { - io_service->poll(); - } -} - } // namespace asiolink } // namespace isc diff --git a/src/lib/asiolink/io_service.h b/src/lib/asiolink/io_service.h index 1800478b78..14a33767d8 100644 --- a/src/lib/asiolink/io_service.h +++ b/src/lib/asiolink/io_service.h @@ -54,7 +54,7 @@ public: /// @brief Start the underlying event loop. /// /// This method does not return control to the caller until - /// the @ref stop() method is called via some handler. + /// the @ref stop() or @ref stopWork() method is called via some handler. void run(); /// @brief Run the underlying event loop for a single event. @@ -120,38 +120,10 @@ public: /// by small bits that are called from time to time). void post(const std::function& callback); - /// @brief Register external IOService. - /// - /// @param io_service The external IOService to be registered. - void registerExternalIOService(IOServicePtr io_service); - - /// @brief Unregister external IOService. - /// - /// @param io_service The external IOService to be unregistered. - void unregisterExternalIOService(IOServicePtr io_service); - - /// @brief Clear the list of external IOService objects. - void clearExternalIOServices() { - external_io_services_.clear(); - } - - /// @brief The count of external IOService objects. - /// - // @return The count of external IOService objects. - size_t externalIOServiceCount() { - return (external_io_services_.size()); - } - - /// @brief Poll external IOService objects. - void pollExternalIOServices(); - private: /// @brief The implementation. boost::shared_ptr io_impl_; - - /// @brief The list of external IOService objects. - std::list external_io_services_; }; } // namespace asiolink diff --git a/src/lib/asiolink/io_service_mgr.cc b/src/lib/asiolink/io_service_mgr.cc new file mode 100644 index 0000000000..6b8f97c577 --- /dev/null +++ b/src/lib/asiolink/io_service_mgr.cc @@ -0,0 +1,40 @@ +// Copyright (C) 2011-2024 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 + +#include + +namespace isc { +namespace asiolink { + +IOServiceMgr& IOServiceMgr::instance() { + static IOServiceMgr instance; + return (instance); +} + +void +IOServiceMgr::registerIOService(IOServicePtr io_service) { + io_services_.push_back(io_service); +} + +void +IOServiceMgr::unregisterIOService(IOServicePtr io_service) { + auto it = std::find(io_services_.begin(), io_services_.end(), io_service); + if (it != io_services_.end()) { + io_services_.erase(it); + } +} + +void +IOServiceMgr::pollIOServices() { + for (auto& io_service : io_services_) { + io_service->poll(); + } +} + +} // namespace asiolink +} // namespace isc diff --git a/src/lib/asiolink/io_service_mgr.h b/src/lib/asiolink/io_service_mgr.h new file mode 100644 index 0000000000..a3b0fb97c2 --- /dev/null +++ b/src/lib/asiolink/io_service_mgr.h @@ -0,0 +1,67 @@ +// Copyright (C) 2024 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 IO_SERVICE_MGR_H +#define IO_SERVICE_MGR_H + +#include + +#include + +namespace isc { +namespace asiolink { + +class IOServiceMgr; + +class IOServiceMgr : boost::noncopyable { +public: + + /// @brief Access the IOServiceMgr singleton instance. + /// + /// @return the singleton instance. + static IOServiceMgr& instance(); + + /// @brief Register IOService. + /// + /// @param io_service The IOService to be registered. + void registerIOService(IOServicePtr io_service); + + /// @brief Unregister IOService. + /// + /// @param io_service The IOService to be unregistered. + void unregisterIOService(IOServicePtr io_service); + + /// @brief Clear the list of IOService objects. + void clearIOServices() { + io_services_.clear(); + } + + /// @brief The count of IOService objects. + /// + // @return The count of IOService objects. + size_t getIOServiceCount() { + return (io_services_.size()); + } + + /// @brief Poll IOService objects. + void pollIOServices(); + +private: + + /// @brief Constructor. + IOServiceMgr() = default; + + /// @brief Destructor. + ~IOServiceMgr() = default; + + /// @brief The list of IOService objects. + std::list io_services_; +}; + +} // namespace asiolink +} // namespace isc + +#endif // IO_SERVICE_MGR_H diff --git a/src/lib/asiolink/tests/Makefile.am b/src/lib/asiolink/tests/Makefile.am index eac8ea73bd..e9e13d57aa 100644 --- a/src/lib/asiolink/tests/Makefile.am +++ b/src/lib/asiolink/tests/Makefile.am @@ -34,6 +34,7 @@ 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_mgr_unittest.cc run_unittests_SOURCES += io_service_signal_unittests.cc run_unittests_SOURCES += io_service_thread_pool_unittests.cc run_unittests_SOURCES += dummy_io_callback_unittest.cc diff --git a/src/lib/asiolink/tests/io_service_mgr_unittest.cc b/src/lib/asiolink/tests/io_service_mgr_unittest.cc new file mode 100644 index 0000000000..d180f66833 --- /dev/null +++ b/src/lib/asiolink/tests/io_service_mgr_unittest.cc @@ -0,0 +1,103 @@ +// Copyright (C) 2024 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 + +#include + +#include + +using namespace isc::asiolink; + +namespace { + +TEST(IOServiceMgr, testIOServiceMgr) { + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 0); + int one_io_callback_count = 0; + auto one_f = [&one_io_callback_count] () { + one_io_callback_count++; + }; + int two_io_callback_count = 0; + auto two_f = [&two_io_callback_count] () { + two_io_callback_count++; + }; + { + IOServicePtr one_io_service(new IOService()); + one_io_service->post(one_f); + + IOServicePtr two_io_service(new IOService()); + two_io_service->post(two_f); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 0); + EXPECT_EQ(two_io_callback_count, 0); + + IOServiceMgr::instance().registerIOService(one_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 1); + + IOServiceMgr::instance().registerIOService(two_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 2); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 1); + EXPECT_EQ(two_io_callback_count, 1); + one_io_service->post(one_f); + two_io_service->post(two_f); + } + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 2); + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 2); + EXPECT_EQ(two_io_callback_count, 2); + + IOServiceMgr::instance().clearIOServices(); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 0); + + IOServicePtr one_io_service(new IOService()); + one_io_service->post(one_f); + + IOServicePtr two_io_service(new IOService()); + two_io_service->post(two_f); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 2); + EXPECT_EQ(two_io_callback_count, 2); + + IOServiceMgr::instance().registerIOService(one_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 1); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 3); + EXPECT_EQ(two_io_callback_count, 2); + one_io_service->post(one_f); + two_io_service->post(two_f); + + IOServiceMgr::instance().registerIOService(two_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 2); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 4); + EXPECT_EQ(two_io_callback_count, 4); + one_io_service->post(one_f); + two_io_service->post(two_f); + + IOServiceMgr::instance().unregisterIOService(one_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 1); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 4); + EXPECT_EQ(two_io_callback_count, 5); + one_io_service->post(one_f); + two_io_service->post(two_f); + + IOServiceMgr::instance().unregisterIOService(two_io_service); + EXPECT_EQ(IOServiceMgr::instance().getIOServiceCount(), 0); + + IOServiceMgr::instance().pollIOServices(); + EXPECT_EQ(one_io_callback_count, 4); + EXPECT_EQ(two_io_callback_count, 5); +} + +} // namespace diff --git a/src/lib/asiolink/tests/io_service_unittest.cc b/src/lib/asiolink/tests/io_service_unittest.cc index 01a550d0f9..7a96702e83 100644 --- a/src/lib/asiolink/tests/io_service_unittest.cc +++ b/src/lib/asiolink/tests/io_service_unittest.cc @@ -45,105 +45,4 @@ TEST(IOService, post) { EXPECT_EQ(3, called[2]); } -TEST(IOService, externalIOService) { - IOServicePtr main_io_service(new IOService()); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 0); - int one_io_callback_count = 0; - auto one_f = [&one_io_callback_count] () { - one_io_callback_count++; - }; - int two_io_callback_count = 0; - auto two_f = [&two_io_callback_count] () { - two_io_callback_count++; - }; - { - IOServicePtr one_io_service(new IOService()); - one_io_service->post(one_f); - - IOServicePtr two_io_service(new IOService()); - two_io_service->post(two_f); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 0); - EXPECT_EQ(two_io_callback_count, 0); - - main_io_service->registerExternalIOService(one_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 1); - - main_io_service->registerExternalIOService(two_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 2); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 1); - EXPECT_EQ(two_io_callback_count, 1); - one_io_service->post(one_f); - two_io_service->post(two_f); - } - EXPECT_EQ(main_io_service->externalIOServiceCount(), 2); - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 2); - EXPECT_EQ(two_io_callback_count, 2); - - main_io_service->clearExternalIOServices(); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 0); - - IOServicePtr one_io_service(new IOService()); - one_io_service->post(one_f); - - IOServicePtr two_io_service(new IOService()); - two_io_service->post(two_f); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 2); - EXPECT_EQ(two_io_callback_count, 2); - - main_io_service->registerExternalIOService(one_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 1); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 3); - EXPECT_EQ(two_io_callback_count, 2); - one_io_service->post(one_f); - two_io_service->post(two_f); - - main_io_service->registerExternalIOService(two_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 2); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 4); - EXPECT_EQ(two_io_callback_count, 4); - one_io_service->post(one_f); - two_io_service->post(two_f); - - main_io_service->unregisterExternalIOService(one_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 1); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 4); - EXPECT_EQ(two_io_callback_count, 5); - one_io_service->post(one_f); - two_io_service->post(two_f); - - main_io_service->unregisterExternalIOService(two_io_service); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 0); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 4); - EXPECT_EQ(two_io_callback_count, 5); - - EXPECT_NO_THROW(main_io_service->registerExternalIOService(main_io_service)); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 1); - main_io_service->post(one_f); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 5); - EXPECT_EQ(two_io_callback_count, 5); - EXPECT_NO_THROW(main_io_service->unregisterExternalIOService(main_io_service)); - EXPECT_EQ(main_io_service->externalIOServiceCount(), 0); - - main_io_service->pollExternalIOServices(); - EXPECT_EQ(one_io_callback_count, 5); - EXPECT_EQ(two_io_callback_count, 5); -} - } diff --git a/src/lib/d2srv/d2_simple_parser.cc b/src/lib/d2srv/d2_simple_parser.cc index 3cd37b532d..c86bcd061d 100644 --- a/src/lib/d2srv/d2_simple_parser.cc +++ b/src/lib/d2srv/d2_simple_parser.cc @@ -6,12 +6,14 @@ #include +#include #include #include #include #include #include +using namespace isc::asiolink; using namespace isc::data; using namespace isc::d2; using namespace isc; @@ -292,6 +294,7 @@ void D2SimpleParser::parse(const D2CfgContextPtr& ctx, // change causes problems when trying to roll back. HooksManager::prepareUnloadLibraries(); static_cast(HooksManager::unloadLibraries()); + IOServiceMgr::instance().clearIOServices(); libraries.loadLibraries(false); } } diff --git a/src/lib/process/d_controller.cc b/src/lib/process/d_controller.cc index 6f2994accb..023b58103a 100644 --- a/src/lib/process/d_controller.cc +++ b/src/lib/process/d_controller.cc @@ -5,6 +5,8 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include + +#include #include #include #include @@ -415,8 +417,6 @@ DControllerBase::configFromFile() { // case of problems. storage->applyLoggingCfg(); - getIOService()->clearExternalIOServices(); - answer = updateConfig(module_config); // In all cases the right logging configuration is in the context. process_->getCfgMgr()->getContext()->applyLoggingCfg(); @@ -657,8 +657,6 @@ DControllerBase::configSetHandler(const std::string&, ConstElementPtr args) { // case of problems. storage->applyLoggingCfg(); - getIOService()->clearExternalIOServices(); - ConstElementPtr answer = updateConfig(module_config); int rcode = 0; parseAnswer(rcode, answer); @@ -848,7 +846,7 @@ DControllerBase::~DControllerBase() { LOG_ERROR(dctl_logger, DCTL_UNLOAD_LIBRARIES_ERROR).arg(msg); } - getIOService()->clearExternalIOServices(); + IOServiceMgr::instance().clearIOServices(); io_signal_set_.reset(); try {