-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2018 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
#include <hooks/hooks_manager.h>
#include <hooks/callout_handle.h>
+#include <map>
+#include <set>
namespace isc {
namespace dhcp {
/// isc::hooks::CalloutHandle object with each request passing through the
/// server. For the DHCP servers, the association is provided by this function.
///
-/// The DHCP servers process a single request at a time. At points where the
-/// CalloutHandle is required, the pointer to the current request (packet) is
-/// passed to this function. If the request is a new one, a pointer to
-/// the request is stored, a new CalloutHandle is allocated (and stored) and
-/// a pointer to the latter object returned to the caller. If the request
-/// matches the one stored, the pointer to the stored CalloutHandle is
-/// returned.
+/// The DHCP servers process requests sequentially, but sometimes packets can
+/// be "parked" waiting for the completion of asynchronous operations scheduled
+/// by hook libraries. While the packets are parked, other packets can be
+/// processed. This poses a requirement for the DHCP server to keep
+/// multiple associations between the packets and their handles. At points where
+/// the CalloutHandle is required, the pointer to the current request (packet)
+/// is passed to this function. If the request is a new one, a pointer to the
+/// request stored, a new CalloutHandle is allocated (and stored) and a pointer
+/// to the latter object returned to the caller. If the request matches one of
+/// the stored requests, the CalloutHandle for this request is returned.
///
-/// A special case is a null pointer being passed. This has the effect of
-/// clearing the stored pointers to the packet being processed and
-/// CalloutHandle. As the stored pointers are shared pointers, clearing them
-/// removes one reference that keeps the pointed-to objects in existence.
+/// In order to avoid the endless growth of the collection of stored associations
+/// this function performs housekeeping of the collection every time it is
+/// called with non-null packet pointer. All entries for which the packet
+/// pointer's reference count is less than 2 are removed. These are the packet
+/// objects which only this function holds reference counts to. Thus, they can
+/// be removed because they are not required by the server anymore. They have
+/// been either dropped or processed.
///
-/// @note If the behaviour of the server changes so that multiple packets can
-/// be active at the same time, this simplistic approach will no longer
-/// be adequate and a more complicated structure (such as a map) will
-/// be needed.
///
+/// A special case is a null pointer being passed to this function. This has
+/// the effect of clearing the pointers to stored handles and packets.
+///
+/// @tparam T Pkt4Ptr or Pkt6Ptr object.
/// @param pktptr Pointer to the packet being processed. This is typically a
/// Pkt4Ptr or Pkt6Ptr object. An empty pointer is passed to clear
/// the stored pointers.
/// CalloutHandle if pktptr points to a packet that has been seen
/// before or a new CalloutHandle if it points to a new one. An empty
/// pointer is returned if pktptr is itself an empty pointer.
-
template <typename T>
isc::hooks::CalloutHandlePtr getCalloutHandle(const T& pktptr) {
- // Stored data is declared static, so is initialized when first accessed
- static T stored_pointer; // Pointer to last packet seen
- static isc::hooks::CalloutHandlePtr stored_handle;
- // Pointer to stored handle
+ isc::hooks::CalloutHandlePtr stored_handle;
+ static std::map<T, isc::hooks::CalloutHandlePtr> store;
if (pktptr) {
- // Pointer given, have we seen it before? (If we have, we don't need to
- // do anything as we will automatically return the stored handle.)
- if (pktptr != stored_pointer) {
+ std::set<T> to_remove;
+
+ // Identify unused handles by checking whether the reference count is
+ // less then 2. This indicates that only our local store keeps the
+ // pointer to the packet.
+ for (auto handle_pair_it = store.begin(); handle_pair_it != store.end();
+ ++handle_pair_it) {
+
+ if (!handle_pair_it->first || handle_pair_it->first.use_count() < 2) {
+ to_remove.insert(handle_pair_it->first);
+ }
+ }
+
+ // Actually remove the unused handles.
+ for (const auto& pktptr : to_remove) {
+ store.erase(pktptr);
+ }
+
+ // Pointer given, have we seen it before?
+ auto stored_handle_it = store.find(pktptr);
+ if (stored_handle_it == store.end()) {
// Not seen before, so store the pointer passed to us and get a new
- // CalloutHandle. (The latter operation frees and probably deletes
- // (depending on other pointers) the stored one.)
- stored_pointer = pktptr;
+ // CalloutHandle.
stored_handle = isc::hooks::HooksManager::createCalloutHandle();
+ store[pktptr] = stored_handle;
+
+ } else {
+ // Return existing pointer.
+ stored_handle = stored_handle_it->second;
}
} else {
-
// Empty pointer passed, clear stored data
- stored_pointer.reset();
- stored_handle.reset();
+ store.clear();
}
return (stored_handle);
EXPECT_TRUE(chptr_1 == chptr_2);
// Reference count is now 3 on the callout handle - two for pointers here,
- // one for the static pointer in the function. The count is 2 for the]
+ // one for the static pointer in the function. The count is 2 for the
// object pointed to by pktptr_1 - one for that pointer and one for the
// pointer in the function.
EXPECT_EQ(3, chptr_1.use_count());
EXPECT_FALSE(chptr_1 == chptr_2);
// Check reference counts. The getCalloutHandle function should be storing
- // pointers to the objects pointed to by chptr_2 and pktptr_2.
+ // pointers to objects pointed to by chptr_1, pktptr_1, chptr_2 and pktptr_2.
+ EXPECT_EQ(2, chptr_1.use_count());
+ EXPECT_EQ(2, pktptr_1.use_count());
+ EXPECT_EQ(2, chptr_2.use_count());
+ EXPECT_EQ(2, pktptr_2.use_count());
+
+ // Now clear the pktptr_1 simulating the situation when the DHCP packet
+ // has been fully processed.
+ pktptr_1.reset();
+
+ // Retrieve chptr_2 again to trigger clearing unused pointer chptr_1.
+ chptr_2 = getCalloutHandle(pktptr_2);
+ ASSERT_TRUE(chptr_2);
+
+ // This should merely affect chptr_1 and pktptr_1, but the pointers for the
+ // second packet should remain unchanged.
EXPECT_EQ(1, chptr_1.use_count());
- EXPECT_EQ(1, pktptr_1.use_count());
EXPECT_EQ(2, chptr_2.use_count());
EXPECT_EQ(2, pktptr_2.use_count());
// Reference counts should be back to 1 for the CalloutHandles and the
// Packet pointers.
EXPECT_EQ(1, chptr_1.use_count());
- EXPECT_EQ(1, pktptr_1.use_count());
EXPECT_EQ(1, chptr_2.use_count());
EXPECT_EQ(1, pktptr_2.use_count());
}
projects / thirdparty / kea.git / commitdiff
