]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[5664] Test that callout handle is reset after the callouts.
authorMarcin Siodelski <marcin@isc.org>
Fri, 29 Jun 2018 13:33:32 +0000 (15:33 +0200)
committerMarcin Siodelski <marcin@isc.org>
Fri, 29 Jun 2018 15:51:41 +0000 (17:51 +0200)
src/bin/dhcp4/dhcp4_srv.cc
src/bin/dhcp4/tests/hooks_unittest.cc
src/bin/dhcp6/dhcp6_srv.cc
src/bin/dhcp6/tests/hooks_unittest.cc
src/lib/dhcpsrv/alloc_engine.cc
src/lib/hooks/callout_handle.cc
src/lib/hooks/callout_handle.h

index dfda5279b672deb260631d05bda3865e60da06a4..4f5c8fa521c480ee7e3c7ef5ba07bca90100c065 100644 (file)
@@ -369,8 +369,11 @@ Dhcpv4Exchange::setHostIdentifiers() {
                 Host::IdentifierType type = Host::IDENT_FLEX;
                 std::vector<uint8_t> id;
 
-                // Delete previously set arguments
-                callout_handle->deleteAllArguments();
+                // 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);
 
                 // Pass incoming packet as argument
                 callout_handle->setArgument("query4", context_->query_);
@@ -530,8 +533,11 @@ Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query, bool& drop,
         HooksManager::calloutsPresent(Hooks.hook_index_subnet4_select_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // We're reusing callout_handle from previous calls
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(query);
@@ -648,8 +654,11 @@ Dhcpv4Srv::selectSubnet4o6(const Pkt4Ptr& query, bool& drop,
         HooksManager::calloutsPresent(Hooks.hook_index_subnet4_select_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // We're reusing callout_handle from previous calls
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Set new arguments
         callout_handle->setArgument("query4", query);
@@ -845,8 +854,11 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) {
     if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(query);
@@ -957,8 +969,11 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) {
     if (HooksManager::calloutsPresent(Hooks.hook_index_pkt4_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(query);
@@ -1041,11 +1056,11 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) {
     if (ctx && HooksManager::calloutsPresent(Hooks.hook_index_leases4_committed_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
-
-        // Clear skip flag if it was set in previous callouts
-        callout_handle->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+        // 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);
 
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(query);
 
@@ -1124,11 +1139,11 @@ Dhcpv4Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle,
     // Execute all callouts registered for pkt4_send
     if (HooksManager::calloutsPresent(Hooks.hook_index_pkt4_send_)) {
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
-
-        // Clear skip flag if it was set in previous callouts
-        callout_handle->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+        // 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);
 
         // Enable copying options from the query and response packets within
         // hook library.
@@ -1183,8 +1198,11 @@ Dhcpv4Srv::processPacketBufferSend(CalloutHandlePtr& callout_handle,
         // Let's execute all callouts registered for buffer4_send
         if (HooksManager::calloutsPresent(Hooks.hook_index_buffer4_send_)) {
 
-            // Delete previously set arguments
-            callout_handle->deleteAllArguments();
+            // 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);
 
             // Enable copying options from the packet within hook library.
             ScopedEnableOptionsCopy<Pkt4> resp4_options_copy(rsp);
@@ -2674,8 +2692,11 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release, AllocEngine::ClientContext4Ptr& cont
         if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_release_)) {
             CalloutHandlePtr callout_handle = getCalloutHandle(release);
 
-            // Delete all previous arguments
-            callout_handle->deleteAllArguments();
+            // 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);
 
             // Enable copying options from the packet within hook library.
             ScopedEnableOptionsCopy<Pkt4> query4_options_copy(release);
@@ -2824,8 +2845,11 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const Pkt4Ptr& decline,
     if (HooksManager::calloutsPresent(Hooks.hook_index_lease4_decline_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(decline);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(decline);
index 1e0898776c632e41b77b6f24cbb5c6ebb541d9e4..f14af29397934eaaf1d60fcaa0c5e21b2aaba18a 100644 (file)
@@ -207,6 +207,21 @@ public:
         return (dis);
     }
 
+    /// @brief Checks if the state of the callout handle associated with a query
+    /// was reset after the callout invocation.
+    ///
+    /// The check includes verification if the status was set to 'continue' and
+    /// that all arguments were deleted.
+    ///
+    /// @param query pointer to the query which callout handle is associated
+    /// with.
+    void checkCalloutHandleReset(const Pkt4Ptr& query) {
+        CalloutHandlePtr callout_handle = query->getCalloutHandle();
+        ASSERT_TRUE(callout_handle);
+        EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle->getStatus());
+        EXPECT_TRUE(callout_handle->getArgumentNames().empty());
+    }
+
     /// Test callback that stores received callout name and pkt4 value
     /// @param callout_handle handle passed by the hooks framework
     /// @return always 0
@@ -909,6 +924,9 @@ TEST_F(HooksDhcpv4SrvTest, Buffer4ReceiveSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(dis);
 }
 
 // Checks if callouts installed on buffer4_receive is able to change
@@ -948,6 +966,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveValueChange) {
     // ... and check if it is the modified value
     ASSERT_FALSE(hwaddr->hwaddr_.empty()); // there must be a MAC address
     EXPECT_EQ(0xff, hwaddr->hwaddr_[0]); // check that its first byte was modified
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on buffer4_receive is able to set skip flag that
@@ -976,6 +997,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveSkip) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on buffer4_receive is able to set drop flag that
@@ -1002,6 +1026,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4ReceiveDrop) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on pkt4_receive are indeed called and the
@@ -1043,6 +1070,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_received is able to change
@@ -1080,6 +1110,9 @@ TEST_F(HooksDhcpv4SrvTest, valueChange_pkt4_receive) {
     // ... and check if it is the modified value
     OptionPtr expected = createOption(DHO_DHCP_CLIENT_IDENTIFIER);
     EXPECT_TRUE(clientid->equals(expected));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_received is able to delete
@@ -1107,6 +1140,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveDeleteClientId) {
 
     // Check that the server dropped the packet and did not send a response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_received is able to set skip flag that
@@ -1133,6 +1169,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveSkip) {
 
     // check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_received is able to set drop flag that
@@ -1159,6 +1198,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4ReceiveDrop) {
 
     // check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 
@@ -1210,6 +1252,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendSimple) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
     EXPECT_TRUE(callback_resp_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_send is able to change
@@ -1247,6 +1292,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendValueChange) {
     // ... and check if it is the modified value
     OptionPtr expected = createOption(DHO_DHCP_SERVER_IDENTIFIER);
     EXPECT_TRUE(clientid->equals(expected));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_send is able to delete
@@ -1282,6 +1330,9 @@ TEST_F(HooksDhcpv4SrvTest, pkt4SendDeleteServerId) {
 
     // Make sure that it does not have server-id
     EXPECT_FALSE(adv->getOption(DHO_DHCP_SERVER_IDENTIFIER));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_skip is able to set skip flag that
@@ -1313,6 +1364,9 @@ TEST_F(HooksDhcpv4SrvTest, skip_pkt4_send) {
     // did not do packing on its own)
     Pkt4Ptr sent = srv_->fake_sent_.front();
     EXPECT_EQ(0, sent->getBuffer().getLength());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt4_drop is able to set drop flag that
@@ -1344,6 +1398,9 @@ TEST_F(HooksDhcpv4SrvTest, drop_pkt4_send) {
     // did not do packing on its own)
     Pkt4Ptr sent = srv_->fake_sent_.front();
     EXPECT_EQ(0, sent->getBuffer().getLength());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer4_send are indeed called and the
@@ -1385,6 +1442,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4SendSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_resp_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on buffer4_send are indeed called and that
@@ -1416,6 +1476,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4Send) {
     // The callout is supposed to fill the output buffer with dummyFile content
     ASSERT_EQ(sizeof(dummyFile), adv->getBuffer().getLength());
     EXPECT_EQ(0, memcmp(adv->getBuffer().getData(), dummyFile, sizeof(dummyFile)));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on buffer4_send can set skip flag and that flag
@@ -1442,6 +1505,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4SendSkip) {
 
     // Check that there is no packet sent.
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 // Checks if callouts installed on buffer4_send can set drop flag and that flag
@@ -1468,6 +1534,9 @@ TEST_F(HooksDhcpv4SrvTest, buffer4SendDrop) {
 
     // Check that there is no packet sent.
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(discover);
 }
 
 
@@ -1548,6 +1617,9 @@ TEST_F(HooksDhcpv4SrvTest, subnet4SelectSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // This test checks if callout installed on subnet4_select hook point can pick
@@ -1615,6 +1687,9 @@ TEST_F(HooksDhcpv4SrvTest, subnet4SelectChange) {
     // in dynamic pool)
     EXPECT_TRUE((*subnets)[1]->inRange(addr));
     EXPECT_TRUE((*subnets)[1]->inPool(Lease::TYPE_V4, addr));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // This test verifies that the leases4_committed hook point is not triggered
@@ -1636,6 +1711,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedDiscover) {
 
     // Make sure that the callout wasn't called.
     EXPECT_TRUE(callback_name_.empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases4_committed hook point is not triggered
@@ -1657,6 +1735,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedInform) {
 
     // Make sure that the callout wasn't called.
     EXPECT_TRUE(callback_name_.empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that incoming (positive) REQUEST/Renewing can be handled
@@ -1755,6 +1836,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that a callout installed on lease4_renew can trigger
@@ -1825,6 +1909,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4RenewSkip) {
     EXPECT_EQ(temp_timestamp, l->cltt_);
 
     EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that the callout installed on the leases4_committed hook
@@ -1867,6 +1954,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Renew the lease and make sure that the callout has been executed.
@@ -1889,6 +1979,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to renew again but force the client to request a different
@@ -1914,6 +2007,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Now request an address that can't be allocated. The client should receive
@@ -1930,6 +2026,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRequest) {
 
     EXPECT_FALSE(callback_lease4_);
     EXPECT_FALSE(callback_deleted_lease4_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that it is possible to park a packet as a result of
@@ -1975,6 +2074,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequests) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client1.getContext().query_);
+
     // Reset all indicators because we'll be now creating a second client.
     resetCalloutBuffers();
 
@@ -2008,6 +2110,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedParkRequests) {
     rsp = client2.getContext().response_;
     EXPECT_EQ(DHCPACK, rsp->getType());
     EXPECT_EQ("192.0.2.101", rsp->getYiaddr().toText());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client2.getContext().query_);
 }
 
 // This test verifies that valid RELEASE triggers lease4_release callouts
@@ -2097,6 +2202,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSimple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(rel);
 }
 
 // This test verifies that skip flag returned by a callout installed on the
@@ -2162,6 +2270,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseSkip) {
     // Try by client-id, should be successful as well.
     leases = LeaseMgrFactory::instance().getLease4(*client_id_);
     EXPECT_EQ(leases.size(), 1);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(rel);
 }
 
 // This test verifies that the leases4_committed callout is executed
@@ -2206,6 +2317,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedRelease) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that drop flag returned by a callout installed on the
@@ -2271,6 +2385,9 @@ TEST_F(HooksDhcpv4SrvTest, lease4ReleaseDrop) {
     // Try by client-id, should be successful as well.
     leases = LeaseMgrFactory::instance().getLease4(*client_id_);
     EXPECT_EQ(leases.size(), 1);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(rel);
 }
 
 // Checks that decline4 hooks (lease4_decline) are triggered properly.
@@ -2320,6 +2437,9 @@ TEST_F(HooksDhcpv4SrvTest, HooksDecline) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Checks that decline4 hook is able to skip the packet.
@@ -2368,6 +2488,9 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineSkip) {
     // lease returned and lease from the lease manager) all match.
     EXPECT_EQ(addr, from_mgr->addr_);
     EXPECT_EQ(addr, callback_lease4_->addr_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Checks that decline4 hook is able to drop the packet.
@@ -2416,6 +2539,9 @@ TEST_F(HooksDhcpv4SrvTest, HooksDeclineDrop) {
     // lease returned and lease from the lease manager) all match.
     EXPECT_EQ(addr, from_mgr->addr_);
     EXPECT_EQ(addr, callback_lease4_->addr_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases4_committed callout is executed
@@ -2461,6 +2587,9 @@ TEST_F(HooksDhcpv4SrvTest, leases4CommittedDecline) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Checks if callout installed on host4_identifier can generate an
@@ -2529,6 +2658,9 @@ TEST_F(HooksDhcpv4SrvTest, host4_identifier) {
 
     // Make sure the address offered is the one that was reserved.
     EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callout installed on host4_identifier can generate identifier of
@@ -2597,6 +2729,9 @@ TEST_F(HooksDhcpv4SrvTest, host4_identifier_hwaddr) {
 
     // Make sure the address offered is the one that was reserved.
     EXPECT_EQ("192.0.2.201", adv->getYiaddr().toText());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 
index 9ed2a3ff7028af160bfcb71067bbddb9de818783..684ab39805eec1ce9ca2900ca74bb31e4bbe8901 100644 (file)
@@ -346,8 +346,11 @@ Dhcpv6Srv::initContext(const Pkt6Ptr& pkt,
                     Host::IdentifierType type = Host::IDENT_FLEX;
                     std::vector<uint8_t> id;
 
-                    // Delete previously set arguments
-                    callout_handle->deleteAllArguments();
+                    // 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);
 
                     // Pass incoming packet as argument
                     callout_handle->setArgument("query6", pkt);
@@ -520,12 +523,15 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) {
     if (HooksManager::calloutsPresent(Hooks.hook_index_buffer6_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
+        // 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);
+
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(query);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
-
         // Pass incoming packet as argument
         callout_handle->setArgument("query6", query);
 
@@ -637,8 +643,11 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) {
     if (HooksManager::calloutsPresent(Hooks.hook_index_pkt6_receive_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(query);
@@ -784,11 +793,11 @@ Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) {
         HooksManager::calloutsPresent(Hooks.hook_index_leases6_committed_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
-
-        // Clear skip flag if it was set in previous callouts
-        callout_handle->setStatus(CalloutHandle::NEXT_STEP_CONTINUE);
+        // 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);
 
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(query);
 
@@ -888,12 +897,15 @@ Dhcpv6Srv::processPacketPktSend(hooks::CalloutHandlePtr& callout_handle,
     // Execute all callouts registered for packet6_send
     if (HooksManager::calloutsPresent(Hooks.hook_index_pkt6_send_)) {
 
+        // 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);
+
         // Enable copying options from the packets within hook library.
         ScopedEnableOptionsCopy<Pkt6> query_resp_options_copy(query, rsp);
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
-
         // Pass incoming packet as argument
         callout_handle->setArgument("query6", query);
 
@@ -948,8 +960,11 @@ Dhcpv6Srv::processPacketBufferSend(CalloutHandlePtr& callout_handle,
         // Let's execute all callouts registered for buffer6_send
         if (HooksManager::calloutsPresent(Hooks.hook_index_buffer6_send_)) {
 
-            // Delete previously set arguments
-            callout_handle->deleteAllArguments();
+            // 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);
 
             // Enable copying options from the packet within hook library.
             ScopedEnableOptionsCopy<Pkt6> response6_options_copy(rsp);
@@ -1354,8 +1369,11 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question, bool& drop) {
     if (HooksManager::calloutsPresent(Hooks.hook_index_subnet6_select_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(question);
 
-        // We're reusing callout_handle from previous calls
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(question);
@@ -2451,6 +2469,12 @@ Dhcpv6Srv::releaseIA_NA(const DuidPtr& duid, const Pkt6Ptr& query,
     if (HooksManager::calloutsPresent(Hooks.hook_index_lease6_release_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
+        // 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);
+
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(query);
 
@@ -2614,8 +2638,11 @@ Dhcpv6Srv::releaseIA_PD(const DuidPtr& duid, const Pkt6Ptr& query,
     if (HooksManager::calloutsPresent(Hooks.hook_index_lease6_release_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(query);
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(query);
@@ -3155,8 +3182,11 @@ Dhcpv6Srv::declineLease(const Pkt6Ptr& decline, const Lease6Ptr lease,
     if (HooksManager::calloutsPresent(Hooks.hook_index_lease6_decline_)) {
         CalloutHandlePtr callout_handle = getCalloutHandle(decline);
 
-        // Delete previously set arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(decline);
index b36b97d680d99acaa6057d8ecfdf24a9648ec2ab..67e88fa597ac8d51d5194ac32569ea49df9ce8cd 100644 (file)
@@ -159,6 +159,21 @@ public:
         return OptionPtr(new Option(Option::V6, option_code, tmp));
     }
 
+    /// @brief Checks if the state of the callout handle associated with a query
+    /// was reset after the callout invocation.
+    ///
+    /// The check includes verification if the status was set to 'continue' and
+    /// that all arguments were deleted.
+    ///
+    /// @param query pointer to the query which callout handle is associated
+    /// with.
+    void checkCalloutHandleReset(const Pkt6Ptr& query) {
+        CalloutHandlePtr callout_handle = query->getCalloutHandle();
+        ASSERT_TRUE(callout_handle);
+        EXPECT_EQ(CalloutHandle::NEXT_STEP_CONTINUE, callout_handle->getStatus());
+        EXPECT_TRUE(callout_handle->getArgumentNames().empty());
+    }
+
     /// test callback that stores received callout name and pkt6 value
     /// @param callout_handle handle passed by the hooks framework
     /// @return always 0
@@ -1027,6 +1042,9 @@ TEST_F(HooksDhcpv6SrvTest, simpleBuffer6Receive) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_receive is able to change
@@ -1063,6 +1081,9 @@ TEST_F(HooksDhcpv6SrvTest, valueChangeBuffer6Receive) {
 
     // ... and check if it is the modified value
     EXPECT_EQ(0xff, clientid->getData()[0]);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_receive is able to delete
@@ -1088,6 +1109,9 @@ TEST_F(HooksDhcpv6SrvTest, deleteClientIdBuffer6Receive) {
 
     // Check that the server dropped the packet and did not send a response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_received is able to set skip flag that
@@ -1112,6 +1136,9 @@ TEST_F(HooksDhcpv6SrvTest, skipBuffer6Receive) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_received is able to set drop flag that
@@ -1136,6 +1163,9 @@ TEST_F(HooksDhcpv6SrvTest, dropBuffer6Receive) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_receive are indeed called and the
@@ -1175,6 +1205,9 @@ TEST_F(HooksDhcpv6SrvTest, simplePkt6Receive) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_received is able to change
@@ -1210,6 +1243,9 @@ TEST_F(HooksDhcpv6SrvTest, valueChangePkt6Receive) {
     // ... and check if it is the modified value
     OptionPtr expected = createOption(D6O_CLIENTID);
     EXPECT_TRUE(clientid->equals(expected));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_received is able to delete
@@ -1235,6 +1271,9 @@ TEST_F(HooksDhcpv6SrvTest, deleteClientIdPkt6Receive) {
 
     // Check that the server dropped the packet and did not send a response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_received is able to set skip flag that
@@ -1259,6 +1298,9 @@ TEST_F(HooksDhcpv6SrvTest, skipPkt6Receive) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_received is able to set drop flag that
@@ -1283,6 +1325,9 @@ TEST_F(HooksDhcpv6SrvTest, dropPkt6Receive) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 
@@ -1327,6 +1372,9 @@ TEST_F(HooksDhcpv6SrvTest, simplePkt6Send) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
     EXPECT_TRUE(callback_resp_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_send is able to change
@@ -1362,6 +1410,9 @@ TEST_F(HooksDhcpv6SrvTest, valueChangePkt6Send) {
     // ... and check if it is the modified value
     OptionPtr expected = createOption(D6O_SERVERID);
     EXPECT_TRUE(clientid->equals(expected));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_send is able to delete
@@ -1395,6 +1446,9 @@ TEST_F(HooksDhcpv6SrvTest, deleteServerIdPkt6Send) {
 
     // Make sure that it does not have server-id
     EXPECT_FALSE(adv->getOption(D6O_SERVERID));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_skip is able to set skip flag that
@@ -1426,6 +1480,9 @@ TEST_F(HooksDhcpv6SrvTest, skipPkt6Send) {
 
     // The actual size of sent packet should be 0
     EXPECT_EQ(0, sent->getBuffer().getLength());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on pkt6_drop is able to set drop flag that
@@ -1450,6 +1507,9 @@ TEST_F(HooksDhcpv6SrvTest, dropPkt6Send) {
 
     // Check that the server does not send the packet
     EXPECT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_send are indeed called and the
@@ -1490,6 +1550,9 @@ TEST_F(HooksDhcpv6SrvTest, simpleBuffer6Send) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_resp_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_send can set skip flag and that flag
@@ -1517,6 +1580,9 @@ TEST_F(HooksDhcpv6SrvTest, buffer6SendSkip) {
 
     // Check that there is no packet sent
     EXPECT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callouts installed on buffer6_send can set drop flag and that flag
@@ -1544,6 +1610,9 @@ TEST_F(HooksDhcpv6SrvTest, buffer6SendDrop) {
 
     // Check that there is no packet sent
     EXPECT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // This test checks if subnet6_select callout is triggered and reports
@@ -1625,6 +1694,9 @@ TEST_F(HooksDhcpv6SrvTest, subnet6Select) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // This test checks if callout installed on subnet6_select hook point can pick
@@ -1703,6 +1775,9 @@ TEST_F(HooksDhcpv6SrvTest, subnet6SselectChange) {
     // in dynamic pool)
     EXPECT_TRUE((*subnets)[1]->inRange(addr_opt->getAddress()));
     EXPECT_TRUE((*subnets)[1]->inPool(Lease::TYPE_NA, addr_opt->getAddress()));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks that subnet6_select is able to drop the packet.
@@ -1726,6 +1801,9 @@ TEST_F(HooksDhcpv6SrvTest, subnet6SelectDrop) {
 
     // Check that the server dropped the packet and did not produce any response
     ASSERT_EQ(0, srv_->fake_sent_.size());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // This test verifies that the leases6_committed hook point is not triggered
@@ -1762,6 +1840,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedSolicit) {
 
     // Make sure that the callout wasn't called.
     EXPECT_TRUE(callback_name_.empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed hook point is not triggered
@@ -1802,6 +1883,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedConfirm) {
 
     // Make sure that the callout wasn't called.
     EXPECT_TRUE(callback_name_.empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed hook point is not triggered
@@ -1837,6 +1921,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedInfRequest) {
 
     // Make sure that the callout wasn't called.
     EXPECT_TRUE(callback_name_.empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -1899,6 +1986,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRapidCommit) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that it is possible to park a SOLICIT packet including
@@ -1971,6 +2061,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRapidCommitPrefixes) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client1.getContext().query_);
+
     // Reset all indicators because we'll be now creating a second client.
     resetCalloutBuffers();
 
@@ -2006,6 +2099,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRapidCommitPrefixes) {
     rsp = client2.getContext().response_;
     EXPECT_EQ(DHCPV6_REPLY, rsp->getType());
     EXPECT_TRUE(client2.hasLeaseForPrefix(IOAddress("2001:db8:1:29::"), 64));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client2.getContext().query_);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -2067,6 +2163,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Request the lease and make sure that the callout has been executed.
@@ -2092,6 +2191,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to request again but force the client to request a different
@@ -2120,6 +2222,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The requested address is just a hint.
@@ -2144,6 +2249,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequest) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Request a prefix: this should lead to an error as no prefix pool
@@ -2165,6 +2273,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequest) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -2230,6 +2341,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequestPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Request the lease and make sure that the callout has been executed.
@@ -2256,6 +2370,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequestPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to request again but force the client to request a different
@@ -2285,6 +2402,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequestPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The requested prefix is just a hint.
@@ -2310,6 +2430,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequestPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Request an address: this should lead to an error as no address pool
@@ -2331,6 +2454,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRequestPrefix) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that it is possible to park a packet as a result of
@@ -2396,6 +2522,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRequests) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client1.getContext().query_);
+
     // Reset all indicators because we'll be now creating a second client.
     resetCalloutBuffers();
 
@@ -2430,6 +2559,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRequests) {
     rsp = client2.getContext().response_;
     EXPECT_EQ(DHCPV6_REPLY, rsp->getType());
     EXPECT_TRUE(client2.hasLeaseForAddress(IOAddress("2001:db8:1::29")));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client2.getContext().query_);
 }
 
 // This test verifies that it is possible to park a packet as a result of
@@ -2499,6 +2631,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRequestsPrefixes) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client1.getContext().query_);
+
     // Reset all indicators because we'll be now creating a second client.
     resetCalloutBuffers();
 
@@ -2533,6 +2668,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedParkRequestsPrefixes) {
     rsp = client2.getContext().response_;
     EXPECT_EQ(DHCPV6_REPLY, rsp->getType());
     EXPECT_TRUE(client2.hasLeaseForPrefix(IOAddress("2001:db8:1:29::"), 64));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client2.getContext().query_);
 }
 
 // This test verifies that incoming (positive) RENEW can be handled properly,
@@ -2634,6 +2772,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Renew) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that incoming (positive) RENEW can be handled properly,
@@ -2726,6 +2867,9 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Renew) {
     EXPECT_GE(1, abs(cltt - expected));
 
     EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr_opt->getAddress()));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that incoming (positive) RENEW can be handled properly,
@@ -2798,6 +2942,9 @@ TEST_F(HooksDhcpv6SrvTest, skipLease6Renew) {
     EXPECT_NE(l->preferred_lft_, subnet_->getPreferred());
     EXPECT_NE(l->valid_lft_, subnet_->getValid());
     EXPECT_NE(l->cltt_, time(NULL));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -2859,6 +3006,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenew) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Renew the lease and make sure that the callout has been executed.
@@ -2912,6 +3062,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenew) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The renewed address is just a hint.
@@ -2936,6 +3089,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenew) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Renew a prefix: this should lead to an error as no prefix pool
@@ -2957,6 +3113,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenew) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -3048,6 +3207,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenewPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to renew again but force the client to renew a different
@@ -3077,6 +3239,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenewPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The renewed prefix is just a hint.
@@ -3102,6 +3267,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenewPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Renew an address: this should lead to an error as no address pool
@@ -3123,6 +3291,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRenewPrefix) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that incoming (positive) RELEASE can be handled properly,
@@ -3208,6 +3379,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Release) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This is a variant of the previous test that tests that callouts are
@@ -3287,6 +3461,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6ReleasePD) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that skip flag returned by a callout installed on the
@@ -3351,6 +3528,9 @@ TEST_F(HooksDhcpv6SrvTest, skipLease6Release) {
     l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, *duid_, iaid,
                                               subnet_->getID());
     ASSERT_TRUE(l);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that drop flag returned by a callout installed on the
@@ -3415,6 +3595,9 @@ TEST_F(HooksDhcpv6SrvTest, dropLease6Release) {
     l = LeaseMgrFactory::instance().getLease6(Lease::TYPE_NA, *duid_, iaid,
                                               subnet_->getID());
     ASSERT_TRUE(l);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that the leases6_committed callout is executed
@@ -3475,6 +3658,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRelease) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed callout is executed
@@ -3539,6 +3725,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedReleasePrefix) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed callout is executed
@@ -3606,6 +3795,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedReleaseMultiple) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that incoming (positive) REBIND can be handled properly,
@@ -3702,6 +3894,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Rebind) {
     // Check that the returned lease6 in callout is the same as the one in the
     // database
     EXPECT_TRUE(*callback_lease6_ == *l);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that incoming (positive) REBIND can be handled properly,
@@ -3791,6 +3986,9 @@ TEST_F(HooksDhcpv6SrvTest, leaseUpdateLease6Rebind) {
     EXPECT_GE(1, abs(cltt - expected));
 
     EXPECT_TRUE(LeaseMgrFactory::instance().deleteLease(addr_opt->getAddress()));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that incoming (positive) REBIND can be handled properly,
@@ -3860,6 +4058,9 @@ TEST_F(HooksDhcpv6SrvTest, skipLease6Rebind) {
     EXPECT_NE(l->preferred_lft_, subnet_->getPreferred());
     EXPECT_NE(l->valid_lft_, subnet_->getValid());
     EXPECT_NE(l->cltt_, time(NULL));
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(req);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -3921,6 +4122,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebind) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Rebind the lease and make sure that the callout has been executed.
@@ -3946,6 +4150,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebind) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to rebind again but force the client to rebind a different
@@ -3974,6 +4181,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebind) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The rebound address is just a hint.
@@ -3998,6 +4208,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebind) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Rebind a prefix: this should lead to an error as no prefix pool
@@ -4019,6 +4232,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebind) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the callout installed on the leases6_committed hook
@@ -4084,6 +4300,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Rebind the lease and make sure that the callout has been executed.
@@ -4110,6 +4329,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Let's try to rebind again but force the client to rebind a different
@@ -4139,6 +4361,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // The rebound prefix is just a hint.
@@ -4164,6 +4389,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
 
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
+
     resetCalloutBuffers();
 
     // Rebind an address: this should lead to an error as no address pool
@@ -4185,6 +4413,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedRebindPrefix) {
     EXPECT_EQ(3, callback_new_leases6_->size());
     ASSERT_TRUE(callback_deleted_leases6_);
     EXPECT_TRUE(callback_deleted_leases6_->empty());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test checks that the basic decline hook (lease6_decline) is
@@ -4236,6 +4467,9 @@ TEST_F(HooksDhcpv6SrvTest, basicLease6Decline) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Test that the lease6_decline hook point can handle SKIP status.
@@ -4283,6 +4517,9 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineSkip) {
     // And that the parameters passed to callout are consistent with the database
     EXPECT_EQ(addr, from_mgr->addr_);
     EXPECT_EQ(addr, callback_lease6_->addr_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Test that the lease6_decline hook point can handle DROP status.
@@ -4328,6 +4565,9 @@ TEST_F(HooksDhcpv6SrvTest, lease6DeclineDrop) {
     ASSERT_TRUE(from_mgr);
     // Now check that it's NOT declined.
     EXPECT_EQ(Lease::STATE_DEFAULT, from_mgr->state_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed callout is executed
@@ -4389,6 +4629,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedDecline) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // This test verifies that the leases6_committed callout is executed
@@ -4453,6 +4696,9 @@ TEST_F(HooksDhcpv6SrvTest, leases6CommittedDeclineTwoNAs) {
 
     // Pkt passed to a callout must be configured to copy retrieved options.
     EXPECT_TRUE(callback_qry_options_copy_);
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(client.getContext().query_);
 }
 
 // Should test with one NA and two addresses but need an example first...
@@ -4532,6 +4778,9 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier) {
 
     ASSERT_TRUE(addr_opt);
     ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 // Checks if callout installed on host6_identifier can generate an identifier
@@ -4609,6 +4858,9 @@ TEST_F(HooksDhcpv6SrvTest, host6Identifier_hwaddr) {
 
     ASSERT_TRUE(addr_opt);
     ASSERT_EQ("2001:db8::f00", addr_opt->getAddress().toText());
+
+    // Check if the callout handle state was reset after the callout.
+    checkCalloutHandleReset(sol);
 }
 
 
index 391915b37d5cc5450642f52466a3d183ea1710be..2ad193492730b2fa75a3ac89d8d023c0cfda6aa6 100644 (file)
@@ -1446,8 +1446,11 @@ AllocEngine::reuseExpiredLease(Lease6Ptr& expired, ClientContext6& ctx,
     if (ctx.callout_handle_ &&
         HooksManager::getHooksManager().calloutsPresent(hook_index_lease6_select_)) {
 
-        // Delete all previous arguments
-        ctx.callout_handle_->deleteAllArguments();
+        // 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(ctx.callout_handle_);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(ctx.query_);
@@ -1532,8 +1535,11 @@ Lease6Ptr AllocEngine::createLease6(ClientContext6& ctx,
     if (ctx.callout_handle_ &&
         HooksManager::getHooksManager().calloutsPresent(hook_index_lease6_select_)) {
 
-        // Delete all previous arguments
-        ctx.callout_handle_->deleteAllArguments();
+        // 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(ctx.callout_handle_);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(ctx.query_);
@@ -1780,8 +1786,11 @@ AllocEngine::extendLease6(ClientContext6& ctx, Lease6Ptr lease) {
     if (HooksManager::calloutsPresent(hook_point)) {
         CalloutHandlePtr callout_handle = ctx.callout_handle_;
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt6> query6_options_copy(ctx.query_);
@@ -2199,6 +2208,13 @@ AllocEngine::reclaimExpiredLease(const Lease6Ptr& lease,
     // will not update DNS nor update the database.
     bool skipped = false;
     if (callout_handle) {
+
+        // 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);
+
         callout_handle->deleteAllArguments();
         callout_handle->setArgument("lease6", lease);
         callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
@@ -2289,7 +2305,13 @@ AllocEngine::reclaimExpiredLease(const Lease4Ptr& lease,
     // will not update DNS nor update the database.
     bool skipped = false;
     if (callout_handle) {
-        callout_handle->deleteAllArguments();
+
+        // 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);
+
         callout_handle->setArgument("lease4", lease);
         callout_handle->setArgument("remove_lease", reclaim_mode == DB_RECLAIM_REMOVE);
 
@@ -2390,8 +2412,11 @@ AllocEngine::reclaimDeclined(const Lease4Ptr& lease) {
             callout_handle = HooksManager::createCalloutHandle();
         }
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Pass necessary arguments
         callout_handle->setArgument("lease4", lease);
@@ -2448,8 +2473,11 @@ AllocEngine::reclaimDeclined(const Lease6Ptr& lease) {
             callout_handle = HooksManager::createCalloutHandle();
         }
 
-        // Delete all previous arguments
-        callout_handle->deleteAllArguments();
+        // 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);
 
         // Pass necessary arguments
         callout_handle->setArgument("lease6", lease);
@@ -3226,8 +3254,11 @@ AllocEngine::createLease4(const ClientContext4& ctx, const IOAddress& addr) {
     if (ctx.callout_handle_ &&
         HooksManager::getHooksManager().calloutsPresent(hook_index_lease4_select_)) {
 
-        // Delete all previous arguments
-        ctx.callout_handle_->deleteAllArguments();
+        // 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(ctx.callout_handle_);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(ctx.query_);
@@ -3331,8 +3362,11 @@ AllocEngine::renewLease4(const Lease4Ptr& lease,
     if (HooksManager::getHooksManager().
         calloutsPresent(Hooks.hook_index_lease4_renew_)) {
 
-        // Delete all previous arguments
-        ctx.callout_handle_->deleteAllArguments();
+        // 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(ctx.callout_handle_);
 
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(ctx.query_);
@@ -3426,8 +3460,11 @@ AllocEngine::reuseExpiredLease4(Lease4Ptr& expired,
         // Enable copying options from the packet within hook library.
         ScopedEnableOptionsCopy<Pkt4> query4_options_copy(ctx.query_);
 
-        // Delete all previous arguments
-        ctx.callout_handle_->deleteAllArguments();
+        // 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(ctx.callout_handle_);
 
         // Pass necessary arguments
         // Pass the original client query
index 8a8e50dd410fdef74c29f69d50154f55ac1c8585..02b3429d825e2ed63887cf291de032070b2fbc52 100644 (file)
@@ -158,7 +158,8 @@ CalloutHandle::getHookName() const {
     return (hook);
 }
 
-ScopedCalloutHandleState::ScopedCalloutHandleState(CalloutHandlePtr& callout_handle)
+ScopedCalloutHandleState::
+ScopedCalloutHandleState(const CalloutHandlePtr& callout_handle)
     : callout_handle_(callout_handle) {
     if (!callout_handle_) {
         isc_throw(BadValue, "callout_handle argument must not be null");
index f421009f5efbb33a9d47ed65ad45e5729c2b7a24..75e5e66ab25649a077ed91e3e0238418a3eeaae8 100644 (file)
@@ -462,7 +462,7 @@ public:
     /// @param callout_handle reference to the pointer to the callout
     /// handle which state should be reset.
     /// @throw isc::BadValue if the callout handle is null.
-    explicit ScopedCalloutHandleState(CalloutHandlePtr& callout_handle);
+    explicit ScopedCalloutHandleState(const CalloutHandlePtr& callout_handle);
 
     /// @brief Destructor.
     ///