[#2473] add continuation for ScopedCalloutHandleState

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 5a5e076bd53a935d94c000e48d6fc759895c4e85..939f55904b200577c209e4f5902698135bd48a8a 100644 (file)
--- 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<ScopedCalloutHandleState> callout_handle_state =
+                std::make_shared<ScopedCalloutHandleState>(callout_handle);
 
         ScopedEnableOptionsCopy<Pkt4> 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<void()> CallBack;
                     boost::shared_ptr<CallBack> call_back =
                         boost::make_shared<CallBack>(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 cac2b442ba91d574bccc0df381982e3b5cecfd26..5b4debb1e371fc700cb57e96c3a5904e83bdc50f 100644 (file)
--- 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<ScopedCalloutHandleState> callout_handle_state =
+                std::make_shared<ScopedCalloutHandleState>(callout_handle);
 
         ScopedEnableOptionsCopy<Pkt6> 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<void()> CallBack;
                 boost::shared_ptr<CallBack> call_back =
                     boost::make_shared<CallBack>(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 cdb2ebc289753a73bfa921ca80e3d2e78afdc38c..6877b65ac6739fec47d92397fbd3c70c4b8395be 100644 (file)
--- 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 b8d92efed1c7fb847436568d3fb59980318711a0..fb4b30aa9369f258b5a997990ab07ce8f8557a2d 100644 (file)
--- 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<void()> on_completion_;
+
 private:
 
     /// @brief Resets the callout handle state.