commit f5eec6e1f995dc7cbb5aab6b083373b991214a67
author Zang MingJie <mingjiez@google.com> Thu Jul 14 21:20:54 2022 +0800
committer GitHub <noreply@github.com> Thu Jul 14 13:20:54 2022 +0000
tree 3e10850e12d188c4b326f7986a96c4ce0a4baa51
parent ad63a6321e3a0ebb0580a7eae1bd46234cc3dd4d

Fix OT UDP receive and UDP close racing (#20536)

* Fix OT UDP receive and UDP close racing

* Add failure log

src/inet/UDPEndPoint.cpp

diff --git a/src/inet/UDPEndPoint.cpp b/src/inet/UDPEndPoint.cpp
index e0907d9..b0b6f3f 100644
--- a/src/inet/UDPEndPoint.cpp
+++ b/src/inet/UDPEndPoint.cpp
@@ -125,8 +125,8 @@
 {
     if (mState != State::kClosed)
     {
-        CloseImpl();
         mState = State::kClosed;
+        CloseImpl();
     }
 }
 

src/inet/UDPEndPointImplLwIP.cpp

diff --git a/src/inet/UDPEndPointImplLwIP.cpp b/src/inet/UDPEndPointImplLwIP.cpp
index 32b10c6..9e6b98f 100644
--- a/src/inet/UDPEndPointImplLwIP.cpp
+++ b/src/inet/UDPEndPointImplLwIP.cpp
@@ -241,6 +241,19 @@
         udp_remove(mUDP);
         mUDP              = nullptr;
         mLwIPEndPointType = LwIPEndPointType::Unknown;
+
+        // In case that there is a UDPEndPointImplLwIP::LwIPReceiveUDPMessage
+        // event pending in the event queue (SystemLayer::ScheduleLambda), we
+        // schedule a release call to the end of the queue, to ensure that the
+        // queued pointer to UDPEndPointImplLwIP is not dangling.
+        Retain();
+        CHIP_ERROR err = GetSystemLayer().ScheduleLambda([this] { Release(); });
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(Inet, "Unable scedule lambda: %" CHIP_ERROR_FORMAT, err.Format());
+            // There is nothing we can do here, accept the chance of racing
+            Release();
+        }
     }
 
     // Unlock LwIP stack
@@ -342,6 +355,10 @@
     UDPEndPointImplLwIP * ep       = static_cast<UDPEndPointImplLwIP *>(arg);
     IPPacketInfo * pktInfo         = nullptr;
     System::PacketBufferHandle buf = System::PacketBufferHandle::Adopt(p);
+
+    if (ep->mState == State::kClosed)
+        return;
+
     if (buf->HasChainedBuffer())
     {
         // Try the simple expedient of flattening in-place.

src/inet/UDPEndPointImplOpenThread.cpp

diff --git a/src/inet/UDPEndPointImplOpenThread.cpp b/src/inet/UDPEndPointImplOpenThread.cpp
index 2f78e80..073f633 100644
--- a/src/inet/UDPEndPointImplOpenThread.cpp
+++ b/src/inet/UDPEndPointImplOpenThread.cpp
@@ -54,6 +54,9 @@
     char destStr[Inet::IPAddress::kMaxStringLength];
 #endif
 
+    if (ep->mState == State::kClosed)
+        return;
+
     if (msgLen > System::PacketBuffer::kMaxSizeWithoutReserve)
     {
         ChipLogError(Inet, "UDP message too long, discarding. Size received %d", msgLen);
@@ -258,6 +261,19 @@
     if (otUdpIsOpen(mOTInstance, &mSocket))
     {
         otUdpClose(mOTInstance, &mSocket);
+
+        // In case that there is a UDPEndPointImplOT::handleUdpReceive event
+        // pending in the event queue (SystemLayer::ScheduleLambda), we
+        // schedule a release call to the end of the queue, to ensure that the
+        // queued pointer to UDPEndPointImplOT is not dangling.
+        Retain();
+        CHIP_ERROR err = GetSystemLayer().ScheduleLambda([this] { Release(); });
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(Inet, "Unable scedule lambda: %" CHIP_ERROR_FORMAT, err.Format());
+            // There is nothing we can do here, accept the chance of racing
+            Release();
+        }
     }
     UnlockOpenThread();
 }