mirror of
https://github.com/espressif/openthread.git
synced 2026-08-01 00:27:47 +00:00
[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.
This commit is contained in:
+30
-3
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user