Skip to content

Commit

Permalink
Make dispatch of OperationalSessionSetup retry notifications safer. (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple authored Oct 31, 2023
1 parent fe513bb commit 5c9d8e8
Showing 1 changed file with 35 additions and 2 deletions.
37 changes: 35 additions & 2 deletions src/app/OperationalSessionSetup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,43 @@ void OperationalSessionSetup::NotifyRetryHandlers(CHIP_ERROR error, const Reliab

void OperationalSessionSetup::NotifyRetryHandlers(CHIP_ERROR error, System::Clock::Seconds16 timeoutEstimate)
{
for (auto * item = mConnectionRetry.First(); item && item != &mConnectionRetry; item = item->mNext)
// We have to be very careful here: Calling into these handlers might in
// theory destroy the Callback objects involved, but unlike the
// succcess/failure cases we don't want to just clear the handlers from our
// list when we are calling them, because we might need to call a given
// handler more than once.
//
// To handle this we:
//
// 1) Snapshot the list of handlers up front, so if any of the handlers
// triggers an AddRetryHandler with some other handler that does not
// affect the list we plan to notify here.
//
// 2) When planning to notify a handler move it to a new list that contains
// just that handler. This way if it gets canceled as part of the
// notification we can tell it has been canceled.
//
// 3) If notifying the handler does not cancel it, add it back to our list
// of handlers so we will notify it on future retries.

Cancelable retryHandlerListSnapshot;
mConnectionRetry.DequeueAll(retryHandlerListSnapshot);

while (retryHandlerListSnapshot.mNext != &retryHandlerListSnapshot)
{
auto cb = Callback::Callback<OnDeviceConnectionRetry>::FromCancelable(item);
auto * cb = Callback::Callback<OnDeviceConnectionRetry>::FromCancelable(retryHandlerListSnapshot.mNext);

Callback::CallbackDeque currentCallbackHolder;
currentCallbackHolder.Enqueue(cb->Cancel());

cb->mCall(cb->mContext, mPeerId, error, timeoutEstimate);

if (currentCallbackHolder.mNext != &currentCallbackHolder)
{
// Callback has not been canceled as part of the call, so is still
// supposed to be registered with us.
AddRetryHandler(cb);
}
}
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_AUTOMATIC_CASE_RETRIES
Expand Down

0 comments on commit 5c9d8e8

Please sign in to comment.