From fd372a900ab5b0c98e397c51a541ff57a7165fcc Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 23 Feb 2026 19:00:35 -0800
Subject: [PATCH] [coap] decouple callback from queue iteration in
 `PendingRequests` (#12528)

This commit updates `PendingRequests::HandleTimer()` and
`PendingRequests::AbortAllMatching()` to perform request finalization
(which invokes user callbacks) outside of the main loop iterating over
the `mRequestMessages` queue.

Iterating over `mRequestMessages` while invoking user callbacks is
unsafe because the callback may modify the request queue (e.g., abort
other transactions), potentially invalidating the iterator. This change
protects against this by moving requests to be finalized into a separate
local `MessageQueue`. The requests in the local queue are then
finalized and freed after the main loop finishes.
---
 src/core/coap/coap.cpp | 33 ++++++++++++++++++++++++++++++---
 src/core/coap/coap.hpp |  1 +
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/core/coap/coap.cpp b/src/core/coap/coap.cpp
index f74268c64..82ac3e3da 100644
--- a/src/core/coap/coap.cpp
+++ b/src/core/coap/coap.cpp
@@ -472,6 +472,7 @@ void CoapBase::PendingRequests::HandleTimer(Timer &aTimer)
 void CoapBase::PendingRequests::HandleTimer(void)
 {
     NextFireTime nextTime;
+    MessageQueue expiredMessages;
 
     for (Message &message : mRequestMessages)
     {
@@ -493,7 +494,14 @@ void CoapBase::PendingRequests::HandleTimer(void)
         {
             if (!request.mMetadata.ShouldRetransmit())
             {
-                FinalizeRequest(request, kErrorResponseTimeout);
+                // We move the expired request to a separate queue to
+                // finalize it after the loop. This ensures that the
+                // iterator over `mRequestMessages` remains valid
+                // even if the user callback (invoked during
+                // finalization) modifies any pending requests
+
+                mRequestMessages.Dequeue(message);
+                expiredMessages.Enqueue(message);
                 continue;
             }
 
@@ -510,6 +518,8 @@ void CoapBase::PendingRequests::HandleTimer(void)
     }
 
     mTimer.FireAt(nextTime);
+
+    FinalizeRemovedRequestsIn(expiredMessages, kErrorResponseTimeout);
 }
 
 void CoapBase::PendingRequests::FinalizeRequest(Request &aRequest, Error aResult)
@@ -1690,7 +1700,8 @@ Error CoapBase::PendingRequests::AbortRequestsMatching(ResponseHandler aHandler,
 
 Error CoapBase::PendingRequests::AbortAllMatching(const Matcher &aMatcher)
 {
-    Error error = kErrorNotFound;
+    Error        error = kErrorNotFound;
+    MessageQueue abortedMessages;
 
     for (Message &message : mRequestMessages)
     {
@@ -1700,14 +1711,30 @@ Error CoapBase::PendingRequests::AbortAllMatching(const Matcher &aMatcher)
 
         if (aMatcher.Matches(request))
         {
-            FinalizeRequest(request, kErrorAbort);
+            mRequestMessages.Dequeue(message);
+            abortedMessages.Enqueue(message);
             error = kErrorNone;
         }
     }
 
+    FinalizeRemovedRequestsIn(abortedMessages, kErrorAbort);
+
     return error;
 }
 
+void CoapBase::PendingRequests::FinalizeRemovedRequestsIn(MessageQueue &aQueue, Error aResult)
+{
+    for (Message &message : aQueue)
+    {
+        Request request;
+
+        request.InitFrom(message);
+        request.mMetadata.mCallbacks.InvokeResponseHandler(/* aResponse */ nullptr, aResult);
+    }
+
+    aQueue.DequeueAndFreeAll();
+}
+
 bool CoapBase::PendingRequests::Matcher::Matches(const Request &aRequest) const
 {
     bool matches = false;
diff --git a/src/core/coap/coap.hpp b/src/core/coap/coap.hpp
index 049db3078..8023f756a 100644
--- a/src/core/coap/coap.hpp
+++ b/src/core/coap/coap.hpp
@@ -873,6 +873,7 @@ private:
         };
 
         Error       AbortAllMatching(const Matcher &aMatcher);
+        void        FinalizeRemovedRequestsIn(MessageQueue &aQueue, Error aResult);
         void        RetransmitRequest(const Request &aRequest);
         static void HandleTimer(Timer &aTimer);
         void        HandleTimer(void);