- Darwin4053
2025-06: Fixed arguments order in flex option hook library debug messages.
+
+ - liyunqing_kylin
+ 2025-09: Reported and provided a patch that fixes a dead lock in ping-check
+ hooks library.
+2404. [bug] razvan, liyunqing_kylin
+ Fixed deadlock in ping-check hooks library. Thanks to
+ liyunqing_kylin for reporting and providing a patch.
+ (Gitlab #4140)
+
Kea 3.1.2 (development) released on September 24, 2025
2403. [func] fdupont
PingChannel::PingChannel(IOServicePtr& io_service,
NextToSendCallback next_to_send_cb,
+ UpdateToSendCallback update_to_send_cb,
EchoSentCallback echo_sent_cb,
ReplyReceivedCallback reply_received_cb,
ShutdownCallback shutdown_cb)
: io_service_(io_service),
next_to_send_cb_(next_to_send_cb),
+ update_to_send_cb_(update_to_send_cb),
echo_sent_cb_(echo_sent_cb),
reply_received_cb_(reply_received_cb),
shutdown_cb_(shutdown_cb),
socket_(0), input_buf_(256),
reading_(false), sending_(false), stopping_(false), mutex_(new std::mutex),
- single_threaded_(!MultiThreadingMgr::instance().getMode()),
+ send_mutex_(new std::mutex), single_threaded_(!MultiThreadingMgr::instance().getMode()),
watch_socket_(0), registered_write_fd_(-1), registered_read_fd_(-1) {
if (!io_service_) {
isc_throw(BadValue,
void
PingChannel::sendNext() {
try {
+ // Mutex used to do atomic read of the store entry using
+ // @ref PingContextStore::getNextToSend and update the context from
+ // WAITING_TO_SEND state to SENDING state using
+ // @ref PingContext::setState. Both functions (when transitioning from
+ // WAITING_TO_SEND state to SENDING state) should be called only
+ // with this mutex locked.
+ MultiThreadingLock send_lock(*send_mutex_);
+
+ // Fetch the next one to send (outside the mutex).
+ PingContextPtr context = ((next_to_send_cb_)());
+ if (!context) {
+ // Nothing to send.
+ return;
+ }
+
MultiThreadingLock lock(*mutex_);
if (!canSend()) {
// Can't send right now, get out.
return;
}
- // Fetch the next one to send.
- IOAddress target("0.0.0.0");
- if (!((next_to_send_cb_)(target))) {
- // Nothing to send.
- return;
+ // Update context to SENDING (inside the mutex).
+ if (update_to_send_cb_) {
+ (update_to_send_cb_)(context);
}
// Have an target IP, build an ECHO REQUEST for it.
sending_ = true;
ICMPMsgPtr next_echo(new ICMPMsg());
next_echo->setType(ICMPMsg::ECHO_REQUEST);
- next_echo->setDestination(target);
+ next_echo->setDestination(context->getTarget());
uint32_t instance_num = nextEchoInstanceNum();
next_echo->setId(static_cast<uint16_t>(instance_num >> 16));
ph::_1, // error
ph::_2)); // bytes_transferred
- ICMPEndpoint target_endpoint(target);
+ ICMPEndpoint target_endpoint(context->getTarget());
asyncSend(echo_icmp.get(), sizeof(struct icmp), &target_endpoint, cb);
} catch (const std::exception& ex) {
// Normal IO failures should be passed to the callback. A failure here
#include <util/watch_socket.h>
#include <icmp_msg.h>
#include <icmp_socket.h>
+#include <ping_context.h>
#include <boost/scoped_ptr.hpp>
#include <boost/enable_shared_from_this.hpp>
/// @brief Defines a pointer to PingSocket.
typedef boost::shared_ptr<PingSocket> PingSocketPtr;
-/// @brief Function type for callback that fetches next IOAddress to ping.
-typedef std::function<bool(asiolink::IOAddress& target)> NextToSendCallback;
+/// @brief Function type for callback to fetch a context with next target to ping.
+typedef std::function<PingContextPtr()> NextToSendCallback;
+
+/// @brief Function type for callback to update a context to SENDING state.
+typedef std::function<void(PingContextPtr context)> UpdateToSendCallback;
/// @brief Function type for callback to invoke upon ECHO send completion.
typedef std::function<void(ICMPMsgPtr& echo, bool send_failed)> EchoSentCallback;
///
/// @param io_service pointer to the IOService instance that will manage
/// the channel's IO. Must not be empty
- /// @param next_to_send_cb callback to invoke to fetch the next IOAddress
- /// to ping
+ /// @param next_to_send_cb callback to invoke to fetch the next context and
+ /// its target address to ping (called outside the mutex)
+ /// @param update_to_send_cb callback to invoke to update the selected
+ /// context to SENDING (called inside the mutex)
/// @param echo_sent_cb callback to invoke when an ECHO send has completed
/// @param reply_received_cb callback to invoke when an ICMP reply has been
/// received. This callback is passed all inbound ICMP messages (e.g. ECHO
/// @throw BadValue if io_service is empty.
PingChannel(asiolink::IOServicePtr& io_service,
NextToSendCallback next_to_send_cb,
+ UpdateToSendCallback update_to_send_cb,
EchoSentCallback echo_sent_cb,
ReplyReceivedCallback reply_received_cb,
ShutdownCallback shutdown_cb = ShutdownCallback());
/// @brief Receive data on the socket asynchronously
///
/// Calls the underlying socket's asyncReceive() method to read a
- /// packet of data from a remote endpoint. Arrival of the data is signalled
+ /// packet of data from a remote endpoint. Arrival of the data is signaled
/// via a call to the callback function.
///
/// This virtual function is provided as means to inject errors during
/// @brief IOService instance the drives socket IO
asiolink::IOServicePtr io_service_;
- /// @brief Callback to invoke to fetch the next address to ping.
+ /// @brief Callback to invoke to fetch the next context with target address to ping.
NextToSendCallback next_to_send_cb_;
+ /// @brief Callback to invoke to update selected context to SENDING state.
+ UpdateToSendCallback update_to_send_cb_;
+
/// @brief Callback to invoke when an ECHO write has completed.
EchoSentCallback echo_sent_cb_;
/// @brief The mutex used to protect internal state.
const boost::scoped_ptr<std::mutex> mutex_;
+ /// @brief The mutex used to protect internal state on send events.
+ ///
+ /// Mutex used to do atomic read of the store entry using
+ /// @ref PingContextStore::getNextToSend and update the context from
+ /// WAITING_TO_SEND state to SENDING state using
+ /// @ref PingContext::setState. Both functions (when transitioning from
+ /// WAITING_TO_SEND state to SENDING state) should be called only
+ /// with this mutex locked.
+ const boost::scoped_ptr<std::mutex> send_mutex_;
+
/// @brief True if channel was opened in single-threaded mode, false
/// otherwise.
bool single_threaded_;
startPing(lease, query, parking_lot, getGlobalConfig());
}
-bool
-PingCheckMgr::nextToSend(IOAddress& next) {
- if (checkSuspended()) {
- return (false);
+PingContextPtr
+PingCheckMgr::nextToSend() {
+ if (!checkSuspended()) {
+ return (store_->getNextToSend());
}
- PingContextPtr context = store_->getNextToSend();
- if (!context) {
- return (false);
- }
+ return (PingContextPtr());
+}
- next = context->getTarget();
+void
+PingCheckMgr::updateContextToSend(PingContextPtr context) {
// Transition to sending.
+ // Must not call @ref PingCheckMgr::checkSuspended() or
+ // it will cause a deadlock.
context->setState(PingContext::SENDING);
store_->updateContext(context);
-
- return (true);
}
void
PingCheckMgr::createChannel(IOServicePtr io_service) {
return (PingChannelPtr(new PingChannel(io_service,
std::bind(&PingCheckMgr::nextToSend,
+ this),
+ std::bind(&PingCheckMgr::updateContextToSend,
this, ph::_1),
std::bind(&PingCheckMgr::sendCompleted,
this, ph::_1, ph::_2),
hooks::ParkingLotHandlePtr& parking_lot);
/// @brief Callback passed to PingChannel to use to retrieve the next
- /// address to check.
+ /// context with address to check.
///
/// Fetches the context which has been in the WAITING_TO_SEND state the
- /// longest and returns its lease address.
+ /// longest and returns it.
///
- /// @param[out] next upon return it will contain the next target address.
- /// Contents are only meaningful if the function returns true.
+ /// @return The context selected to send, or null if none available.
+ virtual PingContextPtr nextToSend();
+
+ /// @brief Callback passed to PingChannel to update a context to SENDING
+ /// state just before sending.
+ ///
+ /// This is invoked inside the channel mutex.
+ /// Must not call @ref PingCheckMgr::checkSuspended() or
+ /// it will cause a deadlock.
///
- /// @return True another target address exists, false otherwise.
- virtual bool nextToSend(asiolink::IOAddress& next);
+ /// @param context The context to update.
+ virtual void updateContextToSend(PingContextPtr context);
/// @brief Callback passed to PingChannel to invoke when an ECHO REQUEST
/// send has completed.
/// only meaningful if the function returns true.
///
/// @return True another target address exists, false otherwise.
- virtual bool nextToSend(IOAddress& next) {
+ virtual PingContextPtr nextToSend() {
if (stopped_) {
- return (false);
+ return (PingContextPtr());
}
MultiThreadingLock lock(*mutex_);
- bool use_next = true;
- if (send_queue_.empty()) {
- use_next = false;
- } else {
- next = send_queue_.front();
+ if (!send_queue_.empty()) {
+ Lease4Ptr lease(new Lease4());
+ lease->addr_ = send_queue_.front();
+ Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1234));
+ return (PingContextPtr(new PingContext(lease, pkt)));
}
stopIfDone();
- return (use_next);
+ return (PingContextPtr());
+ }
+
+ /// @brief Callback to call to update context state (does nothing).
+ virtual void updateContextToSend(PingContextPtr /* context */) {
}
/// @brief Callback to invoke when an ECHO write has completed.
// Create the channel instance with the appropriate io_service.
ASSERT_NO_THROW_LOG(channel_.reset(new TestablePingChannel(
channel_ios,
- std::bind(&PingChannelTest::nextToSend, this, ph::_1),
+ std::bind(&PingChannelTest::nextToSend, this),
+ std::bind(&PingChannelTest::updateContextToSend, this, ph::_1),
std::bind(&PingChannelTest::echoSent, this, ph::_1, ph::_2),
std::bind(&PingChannelTest::replyReceived, this, ph::_1)
)));
// Create the channel instance with the appropriate io_service.
ASSERT_NO_THROW_LOG(channel_.reset(new TestablePingChannel(
channel_ios,
- std::bind(&PingChannelTest::nextToSend, this, ph::_1),
+ std::bind(&PingChannelTest::nextToSend, this),
+ std::bind(&PingChannelTest::updateContextToSend, this, ph::_1),
std::bind(&PingChannelTest::echoSent, this, ph::_1, ph::_2),
std::bind(&PingChannelTest::replyReceived, this, ph::_1),
([this, &shutdown_cb_called]() {
// Create the channel instance.
ASSERT_NO_THROW_LOG(channel_.reset(new TestablePingChannel(
test_io_service_,
- std::bind(&PingChannelTest::nextToSend, this, ph::_1),
+ std::bind(&PingChannelTest::nextToSend, this),
+ std::bind(&PingChannelTest::updateContextToSend, this, ph::_1),
std::bind(&PingChannelTest::echoSent, this, ph::_1, ph::_2),
std::bind(&PingChannelTest::replyReceived, this, ph::_1)
)));
// Create the channel instance.
ASSERT_NO_THROW_LOG(channel_.reset(new TestablePingChannel(
test_io_service_,
- std::bind(&PingChannelTest::nextToSend, this, ph::_1),
+ std::bind(&PingChannelTest::nextToSend, this),
+ std::bind(&PingChannelTest::updateContextToSend, this, ph::_1),
std::bind(&PingChannelTest::echoSent, this, ph::_1, ph::_2),
std::bind(&PingChannelTest::replyReceived, this, ph::_1)
)));
virtual PingChannelPtr createChannel(asiolink::IOServicePtr io_service) {
return (TestablePingChannelPtr(
new TestablePingChannel(io_service,
- std::bind(&PingCheckMgr::nextToSend, this, ph::_1),
- std::bind(&TestablePingCheckMgr::sendCompleted,
- this, ph::_1, ph::_2),
+ std::bind(&PingCheckMgr::nextToSend, this),
+ std::bind(&PingCheckMgr::updateContextToSend, this, ph::_1),
+ std::bind(&TestablePingCheckMgr::sendCompleted, this, ph::_1, ph::_2),
std::bind(&TestablePingCheckMgr::replyReceived, this, ph::_1),
std::bind(&PingCheckMgr::channelShutdown, this))));
}
// ST mode should ignore requested thread number.
createMgr(3, 2, 250, true);
- // Calling nextToSend() should return false.
- IOAddress next("0.0.0.0");
- ASSERT_FALSE(mgr_->nextToSend(next));
+ // Calling nextToSend() should return null.
+ ASSERT_FALSE(mgr_->nextToSend());
// Now let's start 3 contexts.
size_t num_targets = 3;
// Consecutive calls to nextToSend() should return target addresses
// in the order they were created.
+ PingContextPtr ctx;
for (auto const& lqp : lease_query_pairs_) {
// Next to send should return the next address to send.
- ASSERT_TRUE(mgr_->nextToSend(next));
+ ASSERT_TRUE(ctx = mgr_->nextToSend());
+
+ // Update the context state explicitly.
+ ASSERT_NO_THROW(mgr_->updateContextToSend(ctx));
// It should match the lease as created.
- ASSERT_EQ(next, lqp.lease_->addr_);
+ ASSERT_EQ(ctx->getTarget(), lqp.lease_->addr_);
// Fetch the corresponding context.
- PingContextPtr context = getContext(next);
+ PingContextPtr context = getContext(ctx->getTarget());
ASSERT_TRUE(context);
// Verify the state has properly moved to SENDING.
EXPECT_EQ(PingContext::SENDING, context->getState());
}
- // A final call to nextToSend should return false.
- ASSERT_FALSE(mgr_->nextToSend(next));
+ // A final call to nextToSend should return null.
+ ASSERT_FALSE(mgr_->nextToSend());
}
/// @brief Exercises PingCheckMgr::setNextExpiration.
ASSERT_NO_THROW_LOG(mgr_->stop());
ASSERT_TRUE(mgr_->isStopped());
- // Calling nextToSend() should return false.
- IOAddress next("0.0.0.0");
- ASSERT_FALSE(mgr_->nextToSend(next));
+ // Calling nextToSend() should return null.
+ ASSERT_FALSE(mgr_->nextToSend());
// We should have as many declines as we have pairs created.
compareLeaseQueryPairs(declines_);
ASSERT_NO_THROW_LOG(mgr_->stop());
ASSERT_TRUE(mgr_->isStopped());
- // Calling nextToSend() should return false.
- IOAddress next("0.0.0.0");
- ASSERT_FALSE(mgr_->nextToSend(next));
+ // Calling nextToSend() should return null.
+ ASSERT_FALSE(mgr_->nextToSend());
// We should have as many declines as we have pairs created.
compareLeaseQueryPairs(declines_);
/// @throw BadValue if io_service is empty.
TestablePingChannel(asiolink::IOServicePtr& io_service,
NextToSendCallback next_to_send_cb,
+ UpdateToSendCallback update_to_send_cb,
EchoSentCallback echo_sent_cb,
ReplyReceivedCallback reply_received_cb,
ShutdownCallback shutdown_cb = ShutdownCallback())
- : PingChannel(io_service, next_to_send_cb, echo_sent_cb, reply_received_cb, shutdown_cb),
+ : PingChannel(io_service, next_to_send_cb, update_to_send_cb, echo_sent_cb, reply_received_cb, shutdown_cb),
read_number_(0), throw_on_read_number_(0), ec_on_read_number_(0), read_error_ec_(),
write_number_(0), throw_on_write_number_(0), ec_on_write_number_(0), write_error_ec_(),
route_loopback_(true), loopback_map_(), stopped_(false) {
projects / thirdparty / kea.git / commitdiff
