Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[opcreds] Fix LEAVE event on RemoveFabric #18434

Merged
merged 2 commits into from
May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,7 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
static_cast<unsigned>(fabricIndex));
fabricListChanged();

// The Leave event SHOULD be emitted by a Node prior to permanently
// leaving the Fabric.
// The Leave event SHOULD be emitted by a Node prior to permanently leaving the Fabric.
for (auto endpoint : EnabledEndpointsWithServerCluster(Basic::Id))
{
// If Basic cluster is implemented on this endpoint
Expand All @@ -377,6 +376,26 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
ChipLogError(Zcl, "OpCredsFabricTableDelegate: Failed to record Leave event");
}
}

// Try to send the queued events as soon as possible. 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();
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
EventManagement::GetInstance().FabricRemoved(fabricIndex);

// Remove access control entries in reverse order (it could be any order, but reverse order
// will cause less churn in persistent storage).
size_t aclCount = 0;
if (Access::GetAccessControl().GetEntryCount(fabricIndex, aclCount) == CHIP_NO_ERROR)
{
while (aclCount)
{
(void) Access::GetAccessControl().DeleteEntry(nullptr, fabricIndex, --aclCount);
}
}
}

// Gets called when a fabric is loaded into the FabricTable from storage
Expand Down
15 changes: 0 additions & 15 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -333,21 +333,6 @@ class Server
static_cast<unsigned>(fabricIndex), err.Format());
}
}

{
// Remove access control entries in reverse order. (It could be
// any order, but reverse order will cause less churn in
// persistent storage.)
size_t count = 0;
if (Access::GetAccessControl().GetEntryCount(fabricIndex, count) == CHIP_NO_ERROR)
{
while (count)
{
(void) Access::GetAccessControl().DeleteEntry(nullptr, fabricIndex, --count);
}
}
}
app::EventManagement::GetInstance().FabricRemoved(fabricIndex);
};

void OnFabricRetrievedFromStorage(FabricTable & fabricTable, FabricIndex fabricIndex) override
Expand Down