From 4aa48a18e9e2281499fc60b077fb266a8b7c5eef Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Thu, 14 Mar 2024 19:17:06 +0200 Subject: [PATCH] [#3281] use shared_from_this --- src/bin/d2/d2_process.cc | 2 - src/bin/d2/d2_queue_mgr.cc | 5 +- src/bin/d2/tests/d2_queue_mgr_unittests.cc | 45 ++-- src/bin/dhcp6/tests/d2_unittest.cc | 5 +- src/bin/dhcp6/tests/d2_unittest.h | 3 +- .../high_availability/communication_state.cc | 2 - .../dhcp/high_availability/tests/ha_test.cc | 6 +- src/lib/dhcp_ddns/ncr_io.cc | 12 +- src/lib/dhcp_ddns/ncr_io.h | 19 +- src/lib/dhcp_ddns/ncr_udp.cc | 4 +- src/lib/dhcp_ddns/ncr_udp.h | 20 +- src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc | 217 ++++++++++-------- src/lib/dhcpsrv/cfgmgr.cc | 10 +- src/lib/dhcpsrv/cfgmgr.h | 2 +- src/lib/dhcpsrv/d2_client_mgr.cc | 2 +- src/lib/http/client.cc | 4 - 16 files changed, 194 insertions(+), 164 deletions(-) diff --git a/src/bin/d2/d2_process.cc b/src/bin/d2/d2_process.cc index abe075d4f7..8ebc540044 100644 --- a/src/bin/d2/d2_process.cc +++ b/src/bin/d2/d2_process.cc @@ -440,8 +440,6 @@ D2Process::reconfigureQueueMgr() { D2Process::~D2Process() { queue_mgr_->stopListening(); - auto f = [](D2QueueMgrPtr) {}; - getIOService()->post(std::bind(f, queue_mgr_)); } D2CfgMgrPtr diff --git a/src/bin/d2/d2_queue_mgr.cc b/src/bin/d2/d2_queue_mgr.cc index aaf7b6f1a4..d99264b93c 100644 --- a/src/bin/d2/d2_queue_mgr.cc +++ b/src/bin/d2/d2_queue_mgr.cc @@ -108,9 +108,8 @@ D2QueueMgr::initUDPListener(const isc::asiolink::IOAddress& ip_address, // Instantiate a UDP listener and set state to INITTED. // Note UDP listener constructor does not throw. - listener_.reset(new dhcp_ddns:: - NameChangeUDPListener(ip_address, port, format, *this, - reuse_address)); + listener_.reset(new dhcp_ddns::NameChangeUDPListener(ip_address, port, format, + shared_from_this(), reuse_address)); mgr_state_ = INITTED; } diff --git a/src/bin/d2/tests/d2_queue_mgr_unittests.cc b/src/bin/d2/tests/d2_queue_mgr_unittests.cc index 9f57cf4a00..153df5b97d 100644 --- a/src/bin/d2/tests/d2_queue_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_queue_mgr_unittests.cc @@ -201,29 +201,44 @@ bool checkSendVsReceived(NameChangeRequestPtr sent_ncr, (*sent_ncr == *received_ncr)); } +class QueueMgrUDPTestRequestHandler : public NameChangeSender::RequestSendHandler { +public: + /// @brief Constructor + QueueMgrUDPTestRequestHandler() : send_result_(NameChangeSender::SUCCESS) { + } + + /// @brief Implements the send completion handler. + virtual void operator ()(const NameChangeSender::Result result, + NameChangeRequestPtr& ncr) { + // save the result and the NCR sent. + send_result_ = result; + sent_ncrs_.push_back(ncr); + } + + NameChangeSender::Result send_result_; + std::vector sent_ncrs_; +}; + /// @brief Text fixture that allows testing a listener and sender together /// It derives from both the receive and send handler classes and contains /// and instance of UDP listener and UDP sender. -class QueueMgrUDPTest : public virtual ::testing::Test, public D2StatTest, - NameChangeSender::RequestSendHandler { +class QueueMgrUDPTest : public virtual ::testing::Test, public D2StatTest { public: asiolink::IOServicePtr io_service_; NameChangeSenderPtr sender_; isc::asiolink::IntervalTimer test_timer_; D2QueueMgrPtr queue_mgr_; - - NameChangeSender::Result send_result_; - std::vector sent_ncrs_; + boost::shared_ptr handle_; std::vector received_ncrs_; QueueMgrUDPTest() : io_service_(new isc::asiolink::IOService()), test_timer_(io_service_), - send_result_(NameChangeSender::SUCCESS) { + handle_(new QueueMgrUDPTestRequestHandler()) { isc::asiolink::IOAddress addr(TEST_ADDRESS); // Create our sender instance. Note that reuse_address is true. sender_.reset(new NameChangeUDPSender(addr, SENDER_PORT, addr, LISTENER_PORT, - FMT_JSON, *this, 100, true)); + FMT_JSON, handle_, 100, true)); // Set the test timeout to break any running tasks if they hang. test_timer_.setup(std::bind(&QueueMgrUDPTest::testTimeoutHandler, @@ -241,18 +256,10 @@ public: } void reset_results() { - sent_ncrs_.clear(); + handle_->sent_ncrs_.clear(); received_ncrs_.clear(); } - /// @brief Implements the send completion handler. - virtual void operator ()(const NameChangeSender::Result result, - NameChangeRequestPtr& ncr) { - // save the result and the NCR sent. - send_result_ = result; - sent_ncrs_.push_back(ncr); - } - /// @brief Handler invoked when test timeout is hit. /// /// This callback stops all running (hanging) tasks on IO service. @@ -287,13 +294,13 @@ TEST_F (QueueMgrUDPTest, stateModel) { // Verify that initializing the listener moves us to INITTED state. isc::asiolink::IOAddress addr(TEST_ADDRESS); EXPECT_NO_THROW(queue_mgr_->initUDPListener(addr, LISTENER_PORT, - FMT_JSON, true)); + FMT_JSON, true)); EXPECT_EQ(D2QueueMgr::INITTED, queue_mgr_->getMgrState()); // Verify that attempting to initialize the listener, from INITTED // is not allowed. EXPECT_THROW(queue_mgr_->initUDPListener(addr, LISTENER_PORT, - FMT_JSON, true), + FMT_JSON, true), D2QueueMgrError); // Verify that we can enter the RUNNING from INITTED by starting the @@ -360,7 +367,7 @@ TEST_F (QueueMgrUDPTest, liveFeed) { isc::asiolink::IOAddress addr(TEST_ADDRESS); ASSERT_NO_THROW(queue_mgr_->initUDPListener(addr, LISTENER_PORT, - FMT_JSON, true)); + FMT_JSON, true)); ASSERT_EQ(D2QueueMgr::INITTED, queue_mgr_->getMgrState()); ASSERT_NO_THROW(queue_mgr_->startListening()); diff --git a/src/bin/dhcp6/tests/d2_unittest.cc b/src/bin/dhcp6/tests/d2_unittest.cc index 80ba4c967b..7b6ebdb11f 100644 --- a/src/bin/dhcp6/tests/d2_unittest.cc +++ b/src/bin/dhcp6/tests/d2_unittest.cc @@ -25,9 +25,8 @@ namespace test { /// @todo void -D2Dhcpv6Srv::d2ClientErrorHandler(const - dhcp_ddns::NameChangeSender::Result result, - dhcp_ddns::NameChangeRequestPtr& ncr) { +D2Dhcpv6Srv::d2ClientErrorHandler(const dhcp_ddns::NameChangeSender::Result result, + dhcp_ddns::NameChangeRequestPtr& ncr) { ++error_count_; // call base class error handler Dhcpv6Srv::d2ClientErrorHandler(result, ncr); diff --git a/src/bin/dhcp6/tests/d2_unittest.h b/src/bin/dhcp6/tests/d2_unittest.h index 55b94620cc..c74ac3e2be 100644 --- a/src/bin/dhcp6/tests/d2_unittest.h +++ b/src/bin/dhcp6/tests/d2_unittest.h @@ -36,8 +36,7 @@ public: } /// @brief Override the error handler. - virtual void d2ClientErrorHandler(const dhcp_ddns::NameChangeSender:: - Result result, + virtual void d2ClientErrorHandler(const dhcp_ddns::NameChangeSender::Result result, dhcp_ddns::NameChangeRequestPtr& ncr); }; diff --git a/src/hooks/dhcp/high_availability/communication_state.cc b/src/hooks/dhcp/high_availability/communication_state.cc index 2e9d1beae4..38b7566df3 100644 --- a/src/hooks/dhcp/high_availability/communication_state.cc +++ b/src/hooks/dhcp/high_availability/communication_state.cc @@ -229,8 +229,6 @@ void CommunicationState::stopHeartbeatInternal() { if (timer_) { timer_->cancel(); - auto f = [](IntervalTimerPtr) {}; - io_service_->post(std::bind(f, timer_)); timer_.reset(); interval_ = 0; heartbeat_impl_ = 0; diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc index ea0d78f995..2e875065ff 100644 --- a/src/hooks/dhcp/high_availability/tests/ha_test.cc +++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc @@ -82,13 +82,11 @@ HATest::runIOService(long ms) { io_service_->restart(); timer_.reset(new IntervalTimer(io_service_)); timer_->setup(std::bind(&IOService::stop, io_service_), ms, - IntervalTimer::ONE_SHOT); + IntervalTimer::ONE_SHOT); io_service_->run(); timer_->cancel(); - auto f = [](IntervalTimerPtr) {}; - io_service_->post(std::bind(f, timer_)); } void @@ -104,8 +102,6 @@ HATest::runIOService(long ms, std::function stop_condition) { } timer_->cancel(); - auto f = [](IntervalTimerPtr) {}; - io_service_->post(std::bind(f, timer_)); } boost::shared_ptr diff --git a/src/lib/dhcp_ddns/ncr_io.cc b/src/lib/dhcp_ddns/ncr_io.cc index 24c2c70f74..8028091efc 100644 --- a/src/lib/dhcp_ddns/ncr_io.cc +++ b/src/lib/dhcp_ddns/ncr_io.cc @@ -51,7 +51,7 @@ std::string ncrProtocolToString(NameChangeProtocol protocol) { //************************** NameChangeListener *************************** -NameChangeListener::NameChangeListener(RequestReceiveHandler& recv_handler) +NameChangeListener::NameChangeListener(RequestReceiveHandlerPtr recv_handler) : listening_(false), io_pending_(false), recv_handler_(recv_handler) { }; @@ -115,7 +115,7 @@ NameChangeListener::invokeRecvHandler(const Result result, // report it. try { io_pending_ = false; - recv_handler_(result, ncr); + (*recv_handler_)(result, ncr); } catch (const std::exception& ex) { LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_UNCAUGHT_NCR_RECV_HANDLER_ERROR) .arg(ex.what()); @@ -144,7 +144,7 @@ NameChangeListener::invokeRecvHandler(const Result result, NameChangeRequestPtr empty; try { io_pending_ = false; - recv_handler_(ERROR, empty); + (*recv_handler_)(ERROR, empty); } catch (const std::exception& ex) { LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_UNCAUGHT_NCR_RECV_HANDLER_ERROR) @@ -156,7 +156,7 @@ NameChangeListener::invokeRecvHandler(const Result result, //************************* NameChangeSender ****************************** -NameChangeSender::NameChangeSender(RequestSendHandler& send_handler, +NameChangeSender::NameChangeSender(RequestSendHandlerPtr send_handler, size_t send_queue_max) : sending_(false), send_handler_(send_handler), send_queue_max_(send_queue_max), mutex_(new mutex()) { @@ -314,7 +314,7 @@ NameChangeSender::invokeSendHandlerInternal(const NameChangeSender::Result resul // not supposed to throw, but in the event it does we will at least // report it. try { - send_handler_(result, ncr_to_send_); + (*send_handler_)(result, ncr_to_send_); } catch (const std::exception& ex) { LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_UNCAUGHT_NCR_SEND_HANDLER_ERROR) .arg(ex.what()); @@ -343,7 +343,7 @@ NameChangeSender::invokeSendHandlerInternal(const NameChangeSender::Result resul // not supposed to throw, but in the event it does we will at least // report it. try { - send_handler_(ERROR, ncr_to_send_); + (*send_handler_)(ERROR, ncr_to_send_); } catch (const std::exception& ex) { LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_UNCAUGHT_NCR_SEND_HANDLER_ERROR).arg(ex.what()); diff --git a/src/lib/dhcp_ddns/ncr_io.h b/src/lib/dhcp_ddns/ncr_io.h index 59be7e08be..d90ea926a9 100644 --- a/src/lib/dhcp_ddns/ncr_io.h +++ b/src/lib/dhcp_ddns/ncr_io.h @@ -53,6 +53,7 @@ #include #include +#include #include #include @@ -179,7 +180,7 @@ public: /// Applications which will receive NameChangeRequests must provide a /// derivation of this class to the listener constructor in order to /// receive NameChangeRequests. - class RequestReceiveHandler { + class RequestReceiveHandler : public boost::enable_shared_from_this { public: /// @brief Function operator implementing a NCR receive callback. @@ -201,11 +202,14 @@ public: } }; + /// @brief Defines a smart pointer to an instance of a request receive handler. + typedef boost::shared_ptr RequestReceiveHandlerPtr; + /// @brief Constructor /// /// @param recv_handler is a pointer the application layer handler to be /// invoked each time a NCR is received or a receive error occurs. - NameChangeListener(RequestReceiveHandler& recv_handler); + NameChangeListener(RequestReceiveHandlerPtr recv_handler); /// @brief Destructor virtual ~NameChangeListener() { @@ -346,7 +350,7 @@ private: bool io_pending_; /// @brief Application level NCR receive completion handler. - RequestReceiveHandler& recv_handler_; + RequestReceiveHandlerPtr recv_handler_; }; /// @brief Defines a smart pointer to an instance of a listener. @@ -483,7 +487,7 @@ public: /// Applications which will send NameChangeRequests must provide a /// derivation of this class to the sender constructor in order to /// receive send outcome notifications. - class RequestSendHandler { + class RequestSendHandler : public boost::enable_shared_from_this { public: /// @brief Function operator implementing a NCR send callback. @@ -504,6 +508,9 @@ public: } }; + /// @brief Defines a smart pointer to an instance of a request send handler. + typedef boost::shared_ptr RequestSendHandlerPtr; + /// @brief Constructor /// /// @param send_handler is a pointer the application layer handler to be @@ -511,7 +518,7 @@ public: /// @param send_queue_max is the maximum number of entries allowed in the /// send queue. Once the maximum number is reached, all calls to /// sendRequest will fail with an exception. - NameChangeSender(RequestSendHandler& send_handler, + NameChangeSender(RequestSendHandlerPtr send_handler, size_t send_queue_max = MAX_QUEUE_DEFAULT); /// @brief Destructor @@ -831,7 +838,7 @@ private: bool sending_; /// @brief A pointer to registered send completion handler. - RequestSendHandler& send_handler_; + RequestSendHandlerPtr send_handler_; /// @brief Maximum number of entries permitted in the send queue. size_t send_queue_max_; diff --git a/src/lib/dhcp_ddns/ncr_udp.cc b/src/lib/dhcp_ddns/ncr_udp.cc index 80cf4edd85..26579742f7 100644 --- a/src/lib/dhcp_ddns/ncr_udp.cc +++ b/src/lib/dhcp_ddns/ncr_udp.cc @@ -65,7 +65,7 @@ UDPCallback::putData(const uint8_t* src, size_t len) { NameChangeUDPListener:: NameChangeUDPListener(const isc::asiolink::IOAddress& ip_address, const uint32_t port, const NameChangeFormat format, - RequestReceiveHandler& ncr_recv_handler, + RequestReceiveHandlerPtr ncr_recv_handler, const bool reuse_address) : NameChangeListener(ncr_recv_handler), ip_address_(ip_address), port_(port), format_(format), reuse_address_(reuse_address) { @@ -208,7 +208,7 @@ NameChangeUDPSender(const isc::asiolink::IOAddress& ip_address, const uint32_t port, const isc::asiolink::IOAddress& server_address, const uint32_t server_port, const NameChangeFormat format, - RequestSendHandler& ncr_send_handler, + RequestSendHandlerPtr ncr_send_handler, const size_t send_que_max, const bool reuse_address) : NameChangeSender(ncr_send_handler, send_que_max), ip_address_(ip_address), port_(port), server_address_(server_address), diff --git a/src/lib/dhcp_ddns/ncr_udp.h b/src/lib/dhcp_ddns/ncr_udp.h index 37260a21a0..2853428e92 100644 --- a/src/lib/dhcp_ddns/ncr_udp.h +++ b/src/lib/dhcp_ddns/ncr_udp.h @@ -109,6 +109,7 @@ #include #include +#include /// responsibility of the completion handler to perform the steps necessary /// to interpret the raw data provided by the service outcome. The @@ -126,8 +127,7 @@ public: class UDPCallback; /// @brief Defines a function pointer for NameChangeRequest completion handlers. -typedef std::function - UDPCompletionHandler; +typedef std::function UDPCompletionHandler; /// @brief Defines a dynamically allocated shared array. typedef boost::shared_array RawBufferPtr; @@ -162,8 +162,7 @@ public: /// send. /// @param buf_size is the capacity of the buffer /// @param data_source storage for UDP endpoint which supplied the data - Data(RawBufferPtr& buffer, const size_t buf_size, - UDPEndpointPtr& data_source) + Data(RawBufferPtr& buffer, const size_t buf_size, UDPEndpointPtr& data_source) : buffer_(buffer), buf_size_(buf_size), data_source_(data_source), put_len_(0), error_code_(), bytes_transferred_(0) { }; @@ -205,9 +204,9 @@ public: /// /// @throw NcrUDPError if either the handler or buffer pointers /// are invalid. - UDPCallback (RawBufferPtr& buffer, const size_t buf_size, - UDPEndpointPtr& data_source, - const UDPCompletionHandler& handler); + UDPCallback(RawBufferPtr& buffer, const size_t buf_size, + UDPEndpointPtr& data_source, + const UDPCompletionHandler& handler); /// @brief Operator that will be invoked by the asiolink layer. /// @@ -318,8 +317,7 @@ typedef isc::asiolink::UDPSocket NameChangeUDPSocket; class NameChangeUDPListener : public NameChangeListener { public: /// @brief Defines the maximum size packet that can be received. - static const size_t RECV_BUF_MAX = isc::asiolink:: - UDPSocket::MIN_SIZE; + static const size_t RECV_BUF_MAX = isc::asiolink::UDPSocket::MIN_SIZE; /// @brief Constructor /// @@ -336,7 +334,7 @@ public: NameChangeUDPListener(const isc::asiolink::IOAddress& ip_address, const uint32_t port, const NameChangeFormat format, - RequestReceiveHandler& ncr_recv_handler, + RequestReceiveHandlerPtr ncr_recv_handler, const bool reuse_address = false); /// @brief Destructor. @@ -465,7 +463,7 @@ public: NameChangeUDPSender(const isc::asiolink::IOAddress& ip_address, const uint32_t port, const isc::asiolink::IOAddress& server_address, const uint32_t server_port, const NameChangeFormat format, - RequestSendHandler& ncr_send_handler, + RequestSendHandlerPtr ncr_send_handler, const size_t send_que_max = NameChangeSender::MAX_QUEUE_DEFAULT, const bool reuse_address = false); diff --git a/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc b/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc index 0f26b74de6..fb9c7204d3 100644 --- a/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc +++ b/src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc @@ -83,6 +83,9 @@ public: } }; +/// @brief Defines a smart pointer to an instance of a listener handler. +typedef boost::shared_ptr SimpleListenHandlerPtr; + /// @brief Tests the NameChangeUDPListener constructors. /// This test verifies that: /// 1. Given valid parameters, the listener constructor works @@ -90,7 +93,7 @@ TEST(NameChangeUDPListenerBasicTest, constructionTests) { // Verify the default constructor works. IOAddress ip_address(TEST_ADDRESS); uint32_t port = LISTENER_PORT; - SimpleListenHandler ncr_handler; + SimpleListenHandlerPtr ncr_handler(new SimpleListenHandler()); // Verify that valid constructor works. EXPECT_NO_THROW(NameChangeUDPListener(ip_address, port, FMT_JSON, ncr_handler)); @@ -107,7 +110,7 @@ TEST(NameChangeUDPListenerBasicTest, basicListenTests) { IOAddress ip_address(TEST_ADDRESS); uint32_t port = LISTENER_PORT; IOServicePtr io_service(new IOService()); - SimpleListenHandler ncr_handler; + SimpleListenHandlerPtr ncr_handler(new SimpleListenHandler()); NameChangeListenerPtr listener; ASSERT_NO_THROW(listener.reset( @@ -150,14 +153,34 @@ bool checkSendVsReceived(NameChangeRequestPtr sent_ncr, return ((sent_ncr && received_ncr) && (*sent_ncr == *received_ncr)); } +class NameChangeUDPListenerTestHandler : public virtual NameChangeListener::RequestReceiveHandler { +public: + NameChangeListener::Result result_; + NameChangeRequestPtr received_ncr_; + + /// @brief Constructor + NameChangeUDPListenerTestHandler() : result_(NameChangeListener::SUCCESS) { + } + + /// @brief RequestReceiveHandler operator implementation for receiving NCRs. + /// + /// The fixture acts as the "application" layer. It derives from + /// RequestReceiveHandler and as such implements operator() in order to + /// receive NCRs. + virtual void operator ()(const NameChangeListener::Result result, + NameChangeRequestPtr& ncr) { + // save the result and the NCR we received + result_ = result; + received_ncr_ = ncr; + } +}; + /// @brief Text fixture for testing NameChangeUDPListener -class NameChangeUDPListenerTest : public virtual ::testing::Test, - NameChangeListener::RequestReceiveHandler { +class NameChangeUDPListenerTest : public virtual ::testing::Test { public: IOServicePtr io_service_; - NameChangeListener::Result result_; + boost::shared_ptr handle_; NameChangeRequestPtr sent_ncr_; - NameChangeRequestPtr received_ncr_; NameChangeListenerPtr listener_; IntervalTimer test_timer_; @@ -166,12 +189,11 @@ public: // Instantiates the listener member and the test timer. The timer is used // to ensure a test doesn't go awry and hang forever. NameChangeUDPListenerTest() - : io_service_(new IOService()), result_(NameChangeListener::SUCCESS), + : io_service_(new IOService()), handle_(new NameChangeUDPListenerTestHandler()), test_timer_(io_service_) { IOAddress addr(TEST_ADDRESS); listener_.reset(new NameChangeUDPListener(addr, LISTENER_PORT, - FMT_JSON, *this, true)); - + FMT_JSON, handle_, true)); // Set the test timeout to break any running tasks if they hang. test_timer_.setup(std::bind(&NameChangeUDPListenerTest:: testTimeoutHandler, this), @@ -187,7 +209,6 @@ public: } } - /// @brief Converts JSON string into an NCR and sends it to the listener. /// void sendNcr(const std::string& msg) { @@ -215,17 +236,7 @@ public: ncr_buffer.getLength()), listener_endpoint); } - /// @brief RequestReceiveHandler operator implementation for receiving NCRs. - /// - /// The fixture acts as the "application" layer. It derives from - /// RequestReceiveHandler and as such implements operator() in order to - /// receive NCRs. - virtual void operator ()(const NameChangeListener::Result result, - NameChangeRequestPtr& ncr) { - // save the result and the NCR we received - result_ = result; - received_ncr_ = ncr; - } + /// @brief Handler invoked when test timeout is hit /// @@ -258,12 +269,12 @@ TEST_F(NameChangeUDPListenerTest, basicReceiveTests) { EXPECT_NO_THROW(io_service_->runOne()); // Verify the "application" status value for a successful complete. - EXPECT_EQ(NameChangeListener::SUCCESS, result_); + EXPECT_EQ(NameChangeListener::SUCCESS, handle_->result_); // Verify the received request matches the sent request. - EXPECT_TRUE(checkSendVsReceived(sent_ncr_, received_ncr_)) + EXPECT_TRUE(checkSendVsReceived(sent_ncr_, handle_->received_ncr_)) << "sent_ncr_" << (sent_ncr_ ? sent_ncr_->toText() : "") - << "recv_ncr_ " << (received_ncr_ ? received_ncr_->toText() : ""); + << "recv_ncr_ " << (handle_->received_ncr_ ? handle_->received_ncr_->toText() : ""); } // Verify we can gracefully stop listening. @@ -294,6 +305,9 @@ public: int error_count_; }; +/// @brief Defines a smart pointer to an instance of a send handler. +typedef boost::shared_ptr SimpleSendHandlerPtr; + /// @brief Text fixture for testing NameChangeUDPListener class NameChangeUDPSenderBasicTest : public virtual ::testing::Test { public: @@ -317,7 +331,7 @@ public: TEST_F(NameChangeUDPSenderBasicTest, constructionTests) { IOAddress ip_address(TEST_ADDRESS); uint32_t port = SENDER_PORT; - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Verify that constructing with an queue size of zero is not allowed. EXPECT_THROW(NameChangeUDPSender(ip_address, port, @@ -353,7 +367,7 @@ TEST_F(NameChangeUDPSenderBasicTest, constructionTestsMultiThreading) { IOAddress ip_address(TEST_ADDRESS); uint32_t port = SENDER_PORT; - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Verify that constructing with an queue size of zero is not allowed. EXPECT_THROW(NameChangeUDPSender(ip_address, port, @@ -381,7 +395,7 @@ TEST_F(NameChangeUDPSenderBasicTest, constructionTestsMultiThreading) { TEST_F(NameChangeUDPSenderBasicTest, basicSendTests) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -509,7 +523,7 @@ TEST_F(NameChangeUDPSenderBasicTest, basicSendTestsMultiThreading) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -635,7 +649,7 @@ TEST_F(NameChangeUDPSenderBasicTest, basicSendTestsMultiThreading) { TEST_F(NameChangeUDPSenderBasicTest, autoStart) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -690,7 +704,7 @@ TEST_F(NameChangeUDPSenderBasicTest, autoStartMultiThreading) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -742,7 +756,7 @@ TEST_F(NameChangeUDPSenderBasicTest, anyAddressSend) { IOAddress ip_address(TEST_ADDRESS); IOAddress any_address("0.0.0.0"); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -780,7 +794,7 @@ TEST_F(NameChangeUDPSenderBasicTest, anyAddressSendMultiThreading) { IOAddress ip_address(TEST_ADDRESS); IOAddress any_address("0.0.0.0"); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Tests are based on a list of messages, get the count now. int num_msgs = sizeof(valid_msgs)/sizeof(char*); @@ -815,7 +829,7 @@ TEST_F(NameChangeUDPSenderBasicTest, assumeQueue) { IOAddress ip_address(TEST_ADDRESS); uint32_t port = SENDER_PORT; IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); NameChangeRequestPtr ncr; // Tests are based on a list of messages, get the count now. @@ -887,7 +901,7 @@ TEST_F(NameChangeUDPSenderBasicTest, assumeQueueMultiThreading) { IOAddress ip_address(TEST_ADDRESS); uint32_t port = SENDER_PORT; IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); NameChangeRequestPtr ncr; // Tests are based on a list of messages, get the count now. @@ -951,40 +965,71 @@ TEST_F(NameChangeUDPSenderBasicTest, assumeQueueMultiThreading) { ASSERT_THROW(sender2.assumeQueue(sender1), NcrSenderError); } +class NameChangeUDPTestReceiveHandler : public virtual NameChangeListener::RequestReceiveHandler { +public: + /// @brief Constructor + NameChangeUDPTestReceiveHandler() : recv_result_(NameChangeListener::SUCCESS) { + } + + /// @brief Implements the receive completion handler. + virtual void operator ()(const NameChangeListener::Result result, + NameChangeRequestPtr& ncr) { + // save the result and the NCR received. + recv_result_ = result; + received_ncrs_.push_back(ncr); + } + + NameChangeListener::Result recv_result_; + std::vector received_ncrs_; +}; + +class NameChangeUDPTestSendHandler : public virtual NameChangeSender::RequestSendHandler { + public: + /// @brief Constructor + NameChangeUDPTestSendHandler() : send_result_(NameChangeSender::SUCCESS) { + } + + /// @brief Implements the send completion handler. + virtual void operator ()(const NameChangeSender::Result result, + NameChangeRequestPtr& ncr) { + // save the result and the NCR sent. + send_result_ = result; + sent_ncrs_.push_back(ncr); + } + + NameChangeSender::Result send_result_; + std::vector sent_ncrs_; +}; + /// @brief Text fixture that allows testing a listener and sender together /// It derives from both the receive and send handler classes and contains /// and instance of UDP listener and UDP sender. -class NameChangeUDPTest : public virtual ::testing::Test, - NameChangeListener::RequestReceiveHandler, - NameChangeSender::RequestSendHandler { +class NameChangeUDPTest : public virtual ::testing::Test { public: IOServicePtr io_service_; - NameChangeListener::Result recv_result_; - NameChangeSender::Result send_result_; + boost::shared_ptr r_handle_; + boost::shared_ptr s_handle_; NameChangeListenerPtr listener_; - NameChangeSenderPtr sender_; + NameChangeSenderPtr sender_; IntervalTimer test_timer_; - std::vector sent_ncrs_; - std::vector received_ncrs_; - NameChangeUDPTest() - : io_service_(new IOService()), recv_result_(NameChangeListener::SUCCESS), - send_result_(NameChangeSender::SUCCESS), test_timer_(io_service_) { + : io_service_(new IOService()), + r_handle_(new NameChangeUDPTestReceiveHandler()), + s_handle_(new NameChangeUDPTestSendHandler()), + test_timer_(io_service_) { IOAddress addr(TEST_ADDRESS); // Create our listener instance. Note that reuse_address is true. listener_.reset( - new NameChangeUDPListener(addr, LISTENER_PORT, FMT_JSON, - *this, true)); + new NameChangeUDPListener(addr, LISTENER_PORT, FMT_JSON, r_handle_, true)); // Create our sender instance. Note that reuse_address is true. sender_.reset( new NameChangeUDPSender(addr, SENDER_PORT, addr, LISTENER_PORT, - FMT_JSON, *this, 100, true)); + FMT_JSON, s_handle_, 100, true)); // Set the test timeout to break any running tasks if they hang. - test_timer_.setup(std::bind(&NameChangeUDPTest::testTimeoutHandler, - this), + test_timer_.setup(std::bind(&NameChangeUDPTest::testTimeoutHandler, this), TEST_TIMEOUT); // Disable multi-threading MultiThreadingMgr::instance().setMode(false); @@ -1001,25 +1046,13 @@ public: MultiThreadingMgr::instance().setMode(false); } - void reset_results() { - sent_ncrs_.clear(); - received_ncrs_.clear(); - } + void SetUp() { - /// @brief Implements the receive completion handler. - virtual void operator ()(const NameChangeListener::Result result, - NameChangeRequestPtr& ncr) { - // save the result and the NCR received. - recv_result_ = result; - received_ncrs_.push_back(ncr); } - /// @brief Implements the send completion handler. - virtual void operator ()(const NameChangeSender::Result result, - NameChangeRequestPtr& ncr) { - // save the result and the NCR sent. - send_result_ = result; - sent_ncrs_.push_back(ncr); + void reset_results() { + s_handle_->sent_ncrs_.clear(); + r_handle_->received_ncrs_.clear(); } /// @brief Handler invoked when test timeout is hit @@ -1104,7 +1137,7 @@ TEST_F(NameChangeUDPTest, roundTripTest) { } // Execute callbacks until we have sent and received all of messages. - while (sender_->getQueueSize() > 0 || (received_ncrs_.size() < num_msgs)) { + while (sender_->getQueueSize() > 0 || (r_handle_->received_ncrs_.size() < num_msgs)) { EXPECT_NO_THROW(io_service_->runOne()); } @@ -1112,11 +1145,11 @@ TEST_F(NameChangeUDPTest, roundTripTest) { EXPECT_EQ(0, sender_->getQueueSize()); // We should have the same number of sends and receives as we do messages. - ASSERT_EQ(num_msgs, sent_ncrs_.size()); - ASSERT_EQ(num_msgs, received_ncrs_.size()); + ASSERT_EQ(num_msgs, s_handle_->sent_ncrs_.size()); + ASSERT_EQ(num_msgs, r_handle_->received_ncrs_.size()); // Check if the payload was received, ignoring the order if necessary. - checkUnordered(num_msgs, sent_ncrs_, received_ncrs_); + checkUnordered(num_msgs, s_handle_->sent_ncrs_, r_handle_->received_ncrs_); // Verify that we can gracefully stop listening. EXPECT_NO_THROW(listener_->stopListening()); @@ -1158,7 +1191,7 @@ TEST_F(NameChangeUDPTest, roundTripTestMultiThreading) { } // Execute callbacks until we have sent and received all of messages. - while (sender_->getQueueSize() > 0 || (received_ncrs_.size() < num_msgs)) { + while (sender_->getQueueSize() > 0 || (r_handle_->received_ncrs_.size() < num_msgs)) { EXPECT_NO_THROW(io_service_->runOne()); } @@ -1166,12 +1199,12 @@ TEST_F(NameChangeUDPTest, roundTripTestMultiThreading) { EXPECT_EQ(0, sender_->getQueueSize()); // We should have the same number of sends and receives as we do messages. - ASSERT_EQ(num_msgs, sent_ncrs_.size()); - ASSERT_EQ(num_msgs, received_ncrs_.size()); + ASSERT_EQ(num_msgs, s_handle_->sent_ncrs_.size()); + ASSERT_EQ(num_msgs, r_handle_->received_ncrs_.size()); // Verify that what we sent matches what we received. Ignore the order // if necessary. - checkUnordered(num_msgs, sent_ncrs_, received_ncrs_); + checkUnordered(num_msgs, s_handle_->sent_ncrs_, r_handle_->received_ncrs_); // Verify that we can gracefully stop listening. EXPECT_NO_THROW(listener_->stopListening()); @@ -1191,7 +1224,7 @@ TEST_F(NameChangeUDPTest, roundTripTestMultiThreading) { TEST_F(NameChangeUDPSenderBasicTest, watchClosedBeforeSendRequest) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1210,8 +1243,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedBeforeSendRequest) { ASSERT_THROW(sender.sendRequest(ncr), util::WatchSocketError); // Verify we didn't invoke the handler. - EXPECT_EQ(0, ncr_handler.pass_count_); - EXPECT_EQ(0, ncr_handler.error_count_); + EXPECT_EQ(0, ncr_handler->pass_count_); + EXPECT_EQ(0, ncr_handler->error_count_); // Request remains in the queue. Technically it was sent but its // completion handler won't get called. @@ -1226,7 +1259,7 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedBeforeSendRequestMultiThreading) IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1245,8 +1278,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedBeforeSendRequestMultiThreading) ASSERT_THROW(sender.sendRequest(ncr), util::WatchSocketError); // Verify we didn't invoke the handler. - EXPECT_EQ(0, ncr_handler.pass_count_); - EXPECT_EQ(0, ncr_handler.error_count_); + EXPECT_EQ(0, ncr_handler->pass_count_); + EXPECT_EQ(0, ncr_handler->error_count_); // Request remains in the queue. Technically it was sent but its // completion handler won't get called. @@ -1258,7 +1291,7 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedBeforeSendRequestMultiThreading) TEST_F(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequest) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1285,8 +1318,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequest) { // Verify handler got called twice. First request should have be sent // without error, second call should have failed to send due to watch // socket markReady failure. - EXPECT_EQ(1, ncr_handler.pass_count_); - EXPECT_EQ(1, ncr_handler.error_count_); + EXPECT_EQ(1, ncr_handler->pass_count_); + EXPECT_EQ(1, ncr_handler->error_count_); // The second request should still be in the queue. EXPECT_EQ(1, sender.getQueueSize()); @@ -1300,7 +1333,7 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequestMultiThreading) IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1327,8 +1360,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequestMultiThreading) // Verify handler got called twice. First request should have be sent // without error, second call should have failed to send due to watch // socket markReady failure. - EXPECT_EQ(1, ncr_handler.pass_count_); - EXPECT_EQ(1, ncr_handler.error_count_); + EXPECT_EQ(1, ncr_handler->pass_count_); + EXPECT_EQ(1, ncr_handler->error_count_); // The second request should still be in the queue. EXPECT_EQ(1, sender.getQueueSize()); @@ -1340,7 +1373,7 @@ TEST_F(NameChangeUDPSenderBasicTest, watchClosedAfterSendRequestMultiThreading) TEST_F(NameChangeUDPSenderBasicTest, watchSocketBadRead) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1376,8 +1409,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchSocketBadRead) { // Verify handler got called twice. First request should have be sent // without error, second call should have failed to send due to watch // socket markReady failure. - EXPECT_EQ(1, ncr_handler.pass_count_); - EXPECT_EQ(1, ncr_handler.error_count_); + EXPECT_EQ(1, ncr_handler->pass_count_); + EXPECT_EQ(1, ncr_handler->error_count_); // The second request should still be in the queue. EXPECT_EQ(1, sender.getQueueSize()); @@ -1391,7 +1424,7 @@ TEST_F(NameChangeUDPSenderBasicTest, watchSocketBadReadMultiThreading) { IOAddress ip_address(TEST_ADDRESS); IOServicePtr io_service(new IOService()); - SimpleSendHandler ncr_handler; + SimpleSendHandlerPtr ncr_handler(new SimpleSendHandler()); // Create the sender and put into send mode. NameChangeUDPSender sender(ip_address, 0, ip_address, LISTENER_PORT, @@ -1427,8 +1460,8 @@ TEST_F(NameChangeUDPSenderBasicTest, watchSocketBadReadMultiThreading) { // Verify handler got called twice. First request should have be sent // without error, second call should have failed to send due to watch // socket markReady failure. - EXPECT_EQ(1, ncr_handler.pass_count_); - EXPECT_EQ(1, ncr_handler.error_count_); + EXPECT_EQ(1, ncr_handler->pass_count_); + EXPECT_EQ(1, ncr_handler->error_count_); // The second request should still be in the queue. EXPECT_EQ(1, sender.getQueueSize()); diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index c5324046ac..192fd3494b 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -43,7 +43,7 @@ CfgMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) { // Note that D2ClientMgr::setD2Config() actually attempts to apply the // configuration by stopping its sender and opening a new one and so // forth per the new configuration. - d2_client_mgr_.setD2ClientConfig(new_config); + d2_client_mgr_->setD2ClientConfig(new_config); // Manager will throw if the set fails, if it succeeds // we'll update our SrvConfig, configuration_, with the D2ClientConfig @@ -54,17 +54,17 @@ CfgMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) { bool CfgMgr::ddnsEnabled() { - return (d2_client_mgr_.ddnsEnabled()); + return (d2_client_mgr_->ddnsEnabled()); } const D2ClientConfigPtr& CfgMgr::getD2ClientConfig() const { - return (d2_client_mgr_.getD2ClientConfig()); + return (d2_client_mgr_->getD2ClientConfig()); } D2ClientMgr& CfgMgr::getD2ClientMgr() { - return (d2_client_mgr_); + return (*d2_client_mgr_); } void @@ -220,7 +220,7 @@ CfgMgr::mergeIntoCfg(const SrvConfigPtr& target_config, const uint32_t seq) { } CfgMgr::CfgMgr() - : datadir_(DHCP_DATA_DIR, true), d2_client_mgr_(), family_(AF_INET) { + : datadir_(DHCP_DATA_DIR, true), d2_client_mgr_(new D2ClientMgr()), family_(AF_INET) { // DHCP_DATA_DIR must be set set with -DDHCP_DATA_DIR="..." in Makefile.am // Note: the definition of DHCP_DATA_DIR needs to include quotation marks // See AM_CPPFLAGS definition in Makefile.am diff --git a/src/lib/dhcpsrv/cfgmgr.h b/src/lib/dhcpsrv/cfgmgr.h index d34b1c5e87..7265f37c8b 100644 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@ -318,7 +318,7 @@ private: util::Optional datadir_; /// @brief Manages the DHCP-DDNS client and its configuration. - D2ClientMgr d2_client_mgr_; + D2ClientMgrPtr d2_client_mgr_; /// @brief Server configuration /// diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc index bac141e283..575b6f395e 100644 --- a/src/lib/dhcpsrv/d2_client_mgr.cc +++ b/src/lib/dhcpsrv/d2_client_mgr.cc @@ -68,7 +68,7 @@ D2ClientMgr::setD2ClientConfig(D2ClientConfigPtr& new_config) { new_config->getServerIp(), new_config->getServerPort(), new_config->getNcrFormat(), - *this, + shared_from_this(), new_config->getMaxQueueSize())); break; } diff --git a/src/lib/http/client.cc b/src/lib/http/client.cc index 8beac1e6d6..056bcba7b4 100644 --- a/src/lib/http/client.cc +++ b/src/lib/http/client.cc @@ -1327,10 +1327,6 @@ Connection::closeInternal() { tls_socket_->close(); } - auto f = [](IntervalTimerPtr, std::shared_ptr>, - std::shared_ptr>) {}; - io_service_->post(std::bind(f, timer_, tcp_socket_, tls_socket_)); - resetState(); } -- 2.47.2