From e950f05799e86bf7417ce778714793608a0fc9d2 Mon Sep 17 00:00:00 2001 From: liyunqing Date: Tue, 30 Sep 2025 17:10:42 +0800 Subject: [PATCH] [#4140] PingChannel: release lock before invoking PingCheckMgr callback to avoid deadlock Fix a potential deadlock between PingChannel and PingCheckMgr. This patch changes sendNext() to: - Release the channel lock before calling next_to_send_cb_. - Reacquire the lock afterward and re-check canSend() before proceeding. - Keep setting `sending_` inside the locked region to ensure only one send path is active at a time. --- src/hooks/dhcp/ping_check/ping_channel.cc | 27 +++++++++++++++-------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/hooks/dhcp/ping_check/ping_channel.cc b/src/hooks/dhcp/ping_check/ping_channel.cc index 6a6a88c038..eadc3030ba 100644 --- a/src/hooks/dhcp/ping_check/ping_channel.cc +++ b/src/hooks/dhcp/ping_check/ping_channel.cc @@ -310,10 +310,12 @@ PingChannel::startRead() { void PingChannel::sendNext() { try { - MultiThreadingLock lock(*mutex_); - if (!canSend()) { + { + MultiThreadingLock lock(*mutex_); + if (!canSend()) { // Can't send right now, get out. - return; + return; + } } // Fetch the next one to send. @@ -324,14 +326,21 @@ PingChannel::sendNext() { } // 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); + { + MultiThreadingLock lock(*mutex_); + if (!canSend()) { + return; + } - uint32_t instance_num = nextEchoInstanceNum(); - next_echo->setId(static_cast(instance_num >> 16)); - next_echo->setSequence(static_cast(instance_num & 0x0000FFFF)); + sending_ = true; + next_echo->setType(ICMPMsg::ECHO_REQUEST); + next_echo->setDestination(target); + + uint32_t instance_num = nextEchoInstanceNum(); + next_echo->setId(static_cast(instance_num >> 16)); + next_echo->setSequence(static_cast(instance_num & 0x0000FFFF)); + } // Get packed wire-form. ICMPPtr echo_icmp = next_echo->pack();