2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-31 05:55:28 +00:00

[#1733] Parking lot enchancements

src/lib/hooks/parking_lots.h
    ParkingLot now stores objects in a map instead of list to eliminate
    sequential searches

    ParkingLotHandle::park() - now allows parking without a pre-existing
    reference

    ParkingLotHandle::deference() - new method that decrements parked
    object reference counts without invoking their callback

src/lib/hooks/tests/parking_lots_unittest.cc
    TEST(ParkingLotsTest, parkRequireReferenceTests)
    TEST(ParkingLotTest, dereference)
    TEST(ParkingLotTest, multipleObjects) - new tests
This commit is contained in:
Thomas Markwalder
2021-03-31 16:14:26 -04:00
parent 63dc38deae
commit f2ffdb5b68
2 changed files with 221 additions and 42 deletions

View File

@@ -1,4 +1,4 @@
// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2018-2021 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
@@ -14,9 +14,11 @@
#include <functional>
#include <iostream>
#include <sstream>
#include <list>
#include <map>
#include <mutex>
#include <thread>
namespace isc {
namespace hooks {
@@ -60,26 +62,45 @@ namespace hooks {
/// destruction of the parked objects.
class ParkingLot {
public:
/// @brief Parks an object.
///
/// @tparam Type of the parked object.
/// @param parked_object object to be parked, e.g. pointer to a packet.
/// @param unpark_callback callback function to be invoked when the object
/// is unparked.
/// @throw InvalidOperation if the @c reference() wasn't called prior to
/// parking the object.
/// @param require_reference when true, the object to park must have an
/// existing reference in the parking lot. If false, the reference will be
/// be created.
/// @throw InvalidOperation if the @c reference() or @c park() have not been
/// called prior to parking the object.
/// @throw Unexpected if the reference count is not greater than zero. This
/// represents an invalid state and should not happen.
template<typename T>
void park(T parked_object, std::function<void()> unpark_callback) {
void park(T parked_object, std::function<void()> unpark_callback,
bool require_reference = true) {
std::lock_guard<std::mutex> lock(mutex_);
auto it = find(parked_object);
if (it == parking_.end() || it->refcount_ <= 0) {
isc_throw(InvalidOperation, "unable to park an object because"
" reference count for this object hasn't been increased."
" Call ParkingLot::reference() first");
} else {
it->update(parked_object, unpark_callback);
if (it == parking_.end()) {
if (require_reference) {
isc_throw(InvalidOperation, "unable to park an object because"
" reference does not exist.");
}
// Not there add it.
ParkingInfo pinfo(parked_object, unpark_callback);
parking_[makeKey(parked_object)] = pinfo;
return;
}
// Catch the case where it exists but refcount is invalid.
if (it->second.refcount_ <= 0) {
isc_throw(Unexpected, "unable to park an object because"
" reference hasn't been increased.");
}
// Already there, bump the reference count and set the
// the callback.
it->second.update(parked_object, unpark_callback);
}
/// @brief Increases reference counter for the parked object.
@@ -95,13 +116,45 @@ public:
std::lock_guard<std::mutex> lock(mutex_);
auto it = find(parked_object);
if (it == parking_.end()) {
ParkingInfo parking_info(parked_object);
parking_.push_back(parking_info);
// It's not there, add it with a ref count of 1.
ParkingInfo pinfo(parked_object);
parking_[makeKey(parked_object)] = pinfo;
} else {
++it->refcount_;
// It's already there, bump the ref count.
++it->second.refcount_;
}
}
/// @brief Decreases the reference counter for the parked object.
///
/// This method is called by the callouts to decrease the reference count
/// on a parked object. If the reference count reaches zero the object
/// is removed from the parking lot without invoking its callback.
///
/// @tparam Type of the parked object.
/// @param parked_object object which will be parked.
/// @return true if the object was found in the parking lot, false
/// otherwise.
template<typename T>
bool dereference(T parked_object) {
std::lock_guard<std::mutex> lock(mutex_);
auto it = find(parked_object);
if (it == parking_.end()) {
// It's not there, nothing to do.
return (false);
}
// It's there decrement the ref count.
--it->second.refcount_;
if (it->second.refcount_ <= 0) {
// No more references, unpark it.
parking_.erase(it);
}
return (true);
}
/// @brief Signals that the object should be unparked.
///
/// If the specified object is parked in this parking lot, the reference
@@ -128,14 +181,14 @@ public:
}
if (force) {
it->refcount_ = 0;
it->second.refcount_ = 0;
} else {
--it->refcount_;
--it->second.refcount_;
}
if (it->refcount_ <= 0) {
if (it->second.refcount_ <= 0) {
// Unpark the packet and set the callback.
cb = it->unpark_callback_;
cb = it->second.unpark_callback_;
parking_.erase(it);
}
}
@@ -172,7 +225,7 @@ public:
return (false);
}
private:
public:
/// @brief Holds information about parked object.
struct ParkingInfo {
@@ -180,6 +233,11 @@ private:
std::function<void()> unpark_callback_; ///< pointer to the callback
int refcount_; ///< current reference count
/// @brief Constructor.
///
/// Default constructor.
ParkingInfo() {};
/// @brief Constructor.
///
/// @param parked_object object being parked.
@@ -201,8 +259,10 @@ private:
}
};
/// @brief Type of list of parked objects.
typedef std::list<ParkingInfo> ParkingInfoList;
private:
/// @brief Map which stores parked objects.
typedef std::map<std::string, ParkingInfo> ParkingInfoList;
/// @brief Type of the iterator in the list of parked objects.
typedef ParkingInfoList::iterator ParkingInfoListIterator;
@@ -210,20 +270,27 @@ private:
/// @brief Container holding parked objects for this parking lot.
ParkingInfoList parking_;
/// @brief Construct the key for a given parked object.
///
/// @tparam T parked object type.
/// @param parked_object object from which the key should be constructed.
/// @return string containing the object's key.
template<typename T>
std::string makeKey(T parked_object) {
std::stringstream ss;
ss << boost::any_cast<T>(parked_object);
return(ss.str());
}
/// @brief Search for the information about the parked object.
///
/// @tparam T parked object type.
/// @param parked_object object for which the information should be found.
/// @return Iterator pointing to the parked object, or @c parking_.end() if
/// no such object found.
/// @param parked_object object for which to search.
/// @return Iterator pointing to the parked object, or @c parking_.end()
/// if no such object found.
template<typename T>
ParkingInfoListIterator find(T parked_object) {
for (auto it = parking_.begin(); it != parking_.end(); ++it) {
if (boost::any_cast<T>(it->parked_object_) == parked_object) {
return (it);
}
}
return (parking_.end());
return(parking_.find(makeKey(parked_object)));
}
/// @brief The mutex to protect parking lot internal state.
@@ -270,6 +337,21 @@ public:
parking_lot_->reference(parked_object);
}
/// @brief Decreases the reference counter for the parked object.
///
/// This method is called by the callouts to decrease the reference count
/// of a parked object. If the reference count reaches zero the object
/// is removed from the parking lot without invoking its callback.
///
/// @tparam Type of the parked object.
/// @param parked_object object which will be parked.
/// @return true if the object was found in the parking lot, false
/// otherwise.
template<typename T>
bool dereference(T parked_object) {
return (parking_lot_->dereference(parked_object));
}
/// @brief Signals that the object should be unparked.
///
/// If the specified object is parked in this parking lot, the reference

View File

@@ -1,4 +1,4 @@
// Copyright (C) 2018-2020 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2018-2021 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
@@ -9,6 +9,7 @@
#include <exceptions/exceptions.h>
#include <hooks/parking_lots.h>
#include <boost/weak_ptr.hpp>
#include <testutils/gtest_utils.h>
#include <gtest/gtest.h>
#include <string>
@@ -17,6 +18,12 @@ using namespace isc::hooks;
namespace {
// Defines a pointer to a string. The tests use shared pointers
// as parked objects to ensure key matching works correctly with
// them. We're doing this because real-world use parked objects
// are typically pointers to packets.
typedef boost::shared_ptr<std::string> StringPtr;
// Test that it is possible to create and retrieve parking lots for
// specified hook points.
TEST(ParkingLotsTest, createGetParkingLot) {
@@ -41,22 +48,34 @@ TEST(ParkingLotsTest, createGetParkingLot) {
EXPECT_FALSE(parking_lot3 == parking_lot0);
}
// Test that object can't be parked if it hasn't been referenced on the
// parking lot.
TEST(ParkingLotTest, parkWithoutReferencing) {
// Verify that parking objects obey require-reference parameter.
TEST(ParkingLotsTest, parkRequireReferenceTests) {
ParkingLot parking_lot;
std::string parked_object = "foo";
EXPECT_THROW(parking_lot.park(parked_object, [] {
}), InvalidOperation);
// Create object to park.
StringPtr parked_object(new std::string("foo"));
// Cannot park an unreferenced object by default
EXPECT_THROW(parking_lot.park(parked_object, [] {}),
InvalidOperation);
// Cannot park an unreferenced object when require_reference
// parameter is true.
EXPECT_THROW(parking_lot.park(parked_object, [] {}, true),
InvalidOperation);
// Can park an unreferenced object when require_reference
// parameter is false.
EXPECT_NO_THROW(parking_lot.park(parked_object, [] {}, false));
}
// Test that object can be parked and then unparked.
TEST(ParkingLotTest, unpark) {
TEST(ParkingLotsTest, unpark) {
ParkingLotPtr parking_lot = boost::make_shared<ParkingLot>();
ParkingLotHandlePtr parking_lot_handle =
boost::make_shared<ParkingLotHandle>(parking_lot);
std::string parked_object = "foo";
StringPtr parked_object(new std::string("foo"));
// Reference the parked object twice because we're going to test that
// reference counting works fine.
@@ -85,12 +104,12 @@ TEST(ParkingLotTest, unpark) {
}
// Test that parked object can be dropped.
TEST(ParkingLotTest, drop) {
TEST(ParkingLotsTest, drop) {
ParkingLotPtr parking_lot = boost::make_shared<ParkingLot>();
ParkingLotHandlePtr parking_lot_handle =
boost::make_shared<ParkingLotHandle>(parking_lot);
std::string parked_object = "foo";
StringPtr parked_object(new std::string("foo"));
// Reference object twice to test that dropping the packet ignores
// reference counting.
@@ -113,7 +132,7 @@ TEST(ParkingLotTest, drop) {
}
// Test that parked lots can be cleared.
TEST(ParkingLotTest, clear) {
TEST(ParkingLotsTest, clear) {
ParkingLotsPtr parking_lots = boost::make_shared<ParkingLots>();
ParkingLotPtr parking_lot = parking_lots->getParkingLotPtr(1234);
ASSERT_TRUE(parking_lot);
@@ -153,4 +172,82 @@ TEST(ParkingLotTest, clear) {
EXPECT_TRUE(weak_parked_object.expired());
}
// Test that object can be parked and dereferenced.
TEST(ParkingLotsTest, dereference) {
ParkingLotPtr parking_lot = boost::make_shared<ParkingLot>();
ParkingLotHandlePtr parking_lot_handle =
boost::make_shared<ParkingLotHandle>(parking_lot);
StringPtr parked_object(new std::string("foo"));
// Reference the parked object twice because we're going to test that
// reference counting works fine.
ASSERT_NO_THROW(parking_lot_handle->reference(parked_object));
ASSERT_NO_THROW(parking_lot_handle->reference(parked_object));
// This flag will indicate if the callback has been called.
bool unparked = false;
ASSERT_NO_THROW(parking_lot->park(parked_object, [&unparked] {
unparked = true;
}));
// Try to dereference the object. It should decrease the reference count,
// but not unpark the packet yet or invoke the callback.
EXPECT_TRUE(parking_lot_handle->dereference(parked_object));
EXPECT_FALSE(unparked);
// Try to dereference the object. It should decrease the reference count,
// but and unpark the packet but not invoke the callback.
EXPECT_TRUE(parking_lot_handle->dereference(parked_object));
EXPECT_FALSE(unparked);
// Try to dereference the object. It should return false indicating
// the object is no longer parked.
EXPECT_FALSE(parking_lot_handle->dereference(parked_object));
EXPECT_FALSE(unparked);
}
// Verifies that parked objects are correctly distinguished from
// one another.
TEST(ParkingLotsTest, multipleObjects) {
ParkingLotPtr parking_lot = boost::make_shared<ParkingLot>();
ParkingLotHandlePtr parking_lot_handle =
boost::make_shared<ParkingLotHandle>(parking_lot);
// Create an object and park it.
StringPtr object_one(new std::string("one"));
int unparked_one = 0;
ASSERT_NO_THROW(parking_lot->park(object_one, [&unparked_one] {
++unparked_one;
}, false));
// Create a second object and park it.
StringPtr object_two(new std::string("two"));
int unparked_two = 0;
ASSERT_NO_THROW(parking_lot->park(object_two, [&unparked_two] {
++unparked_two;
}, false));
// Create a third object but don't park it.
StringPtr object_three(new std::string("three"));
// Try to unpark object_three. It should fail, and no callbacks
// should get invoked.
EXPECT_FALSE(parking_lot_handle->unpark(object_three));
EXPECT_EQ(unparked_one, 0);
EXPECT_EQ(unparked_two, 0);
// Unpark object one. It should succeed and its callback should
// get invoked.
EXPECT_TRUE(parking_lot_handle->unpark(object_one));
EXPECT_EQ(unparked_one, 1);
EXPECT_EQ(unparked_two, 0);
// Unpark object two. It should succeed and its callback should
// get invoked.
EXPECT_TRUE(parking_lot_handle->unpark(object_two));
EXPECT_EQ(unparked_one, 1);
EXPECT_EQ(unparked_two, 1);
}
}