Skip to content

Commit

Permalink
Fabric removal should only sync-deliver events for that fabric. (#21760)
Browse files Browse the repository at this point in the history
* Fabric removal should only sync-deliver events for that fabric.

We were running into an issue where fabric removal would do sync event
delivery on other fabrics, and then removal of _those_ fabrics could
not deliver events (because there was a Report Data outstanding that
had not gotten a Status Response yet).

The fix is to only sync-deliver events for the fabric about to be
removed when a fabric is removed.

Fixes #21744

* Address review comment.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Dec 18, 2023
1 parent dee20b3 commit 2250768
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -364,13 +364,13 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
}
}

// Try to send the queued events as soon as possible. If the just emitted leave event won't
// Try to send the queued events as soon as possible for this fabric. If the just emitted leave event won't
// be sent this time, it will likely not be delivered at all for the following reasons:
// - removing the fabric expires all associated ReadHandlers, so all subscriptions to
// the leave event will be cancelled.
// - removing the fabric removes all associated access control entries, so generating
// subsequent reports containing the leave event will fail the access control check.
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleUrgentEventDeliverySync();
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleUrgentEventDeliverySync(MakeOptional(fabricIndex));
}

// Gets called when a fabric is deleted
Expand Down
9 changes: 7 additions & 2 deletions src/app/reporting/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -981,14 +981,19 @@ CHIP_ERROR Engine::ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBy
return ScheduleBufferPressureEventDelivery(aBytesWritten);
}

void Engine::ScheduleUrgentEventDeliverySync()
void Engine::ScheduleUrgentEventDeliverySync(Optional<FabricIndex> fabricIndex)
{
InteractionModelEngine::GetInstance()->mReadHandlers.ForEachActiveObject([](ReadHandler * handler) {
InteractionModelEngine::GetInstance()->mReadHandlers.ForEachActiveObject([fabricIndex](ReadHandler * handler) {
if (handler->IsType(ReadHandler::InteractionType::Read))
{
return Loop::Continue;
}

if (fabricIndex.HasValue() && fabricIndex.Value() != handler->GetAccessingFabricIndex())
{
return Loop::Continue;
}

handler->UnblockUrgentEventDelivery();

return Loop::Continue;
Expand Down
8 changes: 7 additions & 1 deletion src/app/reporting/Engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ class Engine

uint64_t GetDirtySetGeneration() const { return mDirtyGeneration; }

void ScheduleUrgentEventDeliverySync();
/**
* Schedule event delivery to happen immediately and run reporting to get
* those reports into messages and on the wire. This can be done either for
* a specific fabric, identified by the provided FabricIndex, or across all
* fabrics if no FabricIndex is provided.
*/
void ScheduleUrgentEventDeliverySync(Optional<FabricIndex> fabricIndex = NullOptional);

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
size_t GetGlobalDirtySetSize() { return mGlobalDirtySet.Allocated(); }
Expand Down

0 comments on commit 2250768

Please sign in to comment.