From: Razvan Becheriu Date: Mon, 4 Jul 2022 19:42:59 +0000 (+0300) Subject: [#2473] add continuation for ScopedCalloutHandleState X-Git-Tag: Kea-2.2.0~84 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=830e5bc713c439bfc4c361dbcf978ba4796c5d1d;p=thirdparty%2Fkea.git [#2473] add continuation for ScopedCalloutHandleState --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 5a5e076bd5..939f55904b 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1420,11 +1420,31 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, CalloutHandlePtr callout_handle = getCalloutHandle(query); if (ctx && HooksManager::calloutsPresent(Hooks.hook_index_leases4_committed_)) { + // The ScopedCalloutHandleState class which guarantees that the task + // is added to the thread pool after the response is reset (if needed) + // and CalloutHandle state is reset. In ST it does nothing. + // A smart pointer is used to store the ScopedCalloutHandleState so that + // a copy of the pointer is created by the lambda and only on the + // destruction of the last reference the task is added. + // In MT there are 2 cases: + // 1. packet is unparked before current thread smart pointer to + // ScopedCalloutHandleState is destroyed: + // - the lamba uses the smart pointer to set the callout which adds the + // task, but the task is added after ScopedCalloutHandleState is + // destroyed, on the destruction of the last reference which is held + // by the current thread. + // 2. packet is unparked after the current thread smart pointer to + // ScopedCalloutHandleState is destroyed: + // - the current thread reference to ScopedCalloutHandleState is + // destroyed, but the reference in the lambda keeps it alive until + // the lamba is called and the last reference is released, at which + // time the task is actually added. // Use the RAII wrapper to make sure that the callout handle state is // reset when this object goes out of scope. All hook points must do // it to prevent possible circular dependency between the callout // handle and its arguments. - ScopedCalloutHandleState callout_handle_state(callout_handle); + std::shared_ptr callout_handle_state = + std::make_shared(callout_handle); ScopedEnableOptionsCopy query4_options_copy(query); @@ -1477,13 +1497,15 @@ Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, // NEXT_STEP_PARK. Otherwise the callback we bind here will be // executed when the hook library unparks the packet. HooksManager::park("leases4_committed", query, - [this, callout_handle, query, rsp]() mutable { + [this, callout_handle, query, rsp, callout_handle_state]() mutable { if (MultiThreadingMgr::instance().getMode()) { typedef function CallBack; boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv4Srv::sendResponseNoThrow, this, callout_handle, query, rsp)); - MultiThreadingMgr::instance().getThreadPool().add(call_back); + callout_handle_state->on_completion_ = [call_back]() { + MultiThreadingMgr::instance().getThreadPool().add(call_back); + }; } else { processPacketPktSend(callout_handle, query, rsp); processPacketBufferSend(callout_handle, rsp); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index cac2b442ba..5b4debb1e3 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -1054,12 +1054,31 @@ Dhcpv6Srv::processDhcp6Query(Pkt6Ptr& query, Pkt6Ptr& rsp) { if (!ctx.fake_allocation_ && (ctx.query_->getType() != DHCPV6_CONFIRM) && (ctx.query_->getType() != DHCPV6_INFORMATION_REQUEST) && HooksManager::calloutsPresent(Hooks.hook_index_leases6_committed_)) { - + // The ScopedCalloutHandleState class which guarantees that the task + // is added to the thread pool after the response is reset (if needed) + // and CalloutHandle state is reset. In ST it does nothing. + // A smart pointer is used to store the ScopedCalloutHandleState so that + // a copy of the pointer is created by the lambda and only on the + // destruction of the last reference the task is added. + // In MT there are 2 cases: + // 1. packet is unparked before current thread smart pointer to + // ScopedCalloutHandleState is destroyed: + // - the lamba uses the smart pointer to set the callout which adds the + // task, but the task is added after ScopedCalloutHandleState is + // destroyed, on the destruction of the last reference which is held + // by the current thread. + // 2. packet is unparked after the current thread smart pointer to + // ScopedCalloutHandleState is destroyed: + // - the current thread reference to ScopedCalloutHandleState is + // destroyed, but the reference in the lambda keeps it alive until + // the lamba is called and the last reference is released, at which + // time the task is actually added. // Use the RAII wrapper to make sure that the callout handle state is // reset when this object goes out of scope. All hook points must do // it to prevent possible circular dependency between the callout // handle and its arguments. - ScopedCalloutHandleState callout_handle_state(callout_handle); + std::shared_ptr callout_handle_state = + std::make_shared(callout_handle); ScopedEnableOptionsCopy query6_options_copy(query); @@ -1132,13 +1151,15 @@ Dhcpv6Srv::processDhcp6Query(Pkt6Ptr& query, Pkt6Ptr& rsp) { // NEXT_STEP_PARK. Otherwise the callback we bind here will be // executed when the hook library unparks the packet. HooksManager::park("leases6_committed", query, - [this, callout_handle, query, rsp]() mutable { + [this, callout_handle, query, rsp, callout_handle_state]() mutable { if (MultiThreadingMgr::instance().getMode()) { typedef function CallBack; boost::shared_ptr call_back = boost::make_shared(std::bind(&Dhcpv6Srv::sendResponseNoThrow, this, callout_handle, query, rsp)); - MultiThreadingMgr::instance().getThreadPool().add(call_back); + callout_handle_state->on_completion_ = [call_back]() { + MultiThreadingMgr::instance().getThreadPool().add(call_back); + }; } else { processPacketPktSend(callout_handle, query, rsp); processPacketBufferSend(callout_handle, rsp); diff --git a/src/lib/hooks/callout_handle.cc b/src/lib/hooks/callout_handle.cc index cdb2ebc289..6877b65ac6 100644 --- a/src/lib/hooks/callout_handle.cc +++ b/src/lib/hooks/callout_handle.cc @@ -149,6 +149,10 @@ ScopedCalloutHandleState(const CalloutHandlePtr& callout_handle) ScopedCalloutHandleState::~ScopedCalloutHandleState() { resetState(); + + if (on_completion_) { + on_completion_(); + } } void diff --git a/src/lib/hooks/callout_handle.h b/src/lib/hooks/callout_handle.h index b8d92efed1..fb4b30aa93 100644 --- a/src/lib/hooks/callout_handle.h +++ b/src/lib/hooks/callout_handle.h @@ -491,6 +491,9 @@ public: /// Resets state of the callout handle. ~ScopedCalloutHandleState(); + /// @brief Continuation callback. + std::function on_completion_; + private: /// @brief Resets the callout handle state.