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

remove read client/handler and write handler when corresponding fabric is removed #21204

Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
address comments
yunhanw-google committed Aug 11, 2022
commit f83c21b4685f7d5a5159513acdfa1cfeea186d75
28 changes: 6 additions & 22 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
@@ -77,9 +77,9 @@ void InteractionModelEngine::Shutdown()
//
while (handlerIter)
{
CommandHandlerInterface * pNext = handlerIter->GetNext();
CommandHandlerInterface * nextHandler = handlerIter->GetNext();
handlerIter->SetNext(nullptr);
handlerIter = pNext;
handlerIter = nextHandler;
}

mCommandHandlerList = nullptr;
@@ -236,24 +236,6 @@ uint32_t InteractionModelEngine::GetNumActiveWriteHandlers() const
return numActive;
}

void InteractionModelEngine::CloseTransactionsFromFabricIndex(FabricIndex aFabricIndex)
{
//
// Walk through all existing subscriptions and shut down those whose subscriber matches
// that which just came in.
//
mReadHandlers.ForEachActiveObject([this, aFabricIndex](ReadHandler * handler) {
if (handler->GetAccessingFabricIndex() == aFabricIndex)
{
ChipLogProgress(InteractionModel, "Deleting expired ReadHandler for NodeId: " ChipLogFormatX64 ", FabricIndex: %u",
ChipLogValueX64(handler->GetInitiatorNodeId()), aFabricIndex);
mReadHandlers.ReleaseObject(handler);
}

return Loop::Continue;
});
}

CHIP_ERROR InteractionModelEngine::ShutdownSubscription(SubscriptionId aSubscriptionId)
{
for (auto * readClient = mpActiveReadClientList; readClient != nullptr; readClient = readClient->GetNextClient())
@@ -1440,7 +1422,8 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
mReadHandlers.ForEachActiveObject([fabricIndex](ReadHandler * handler) {
if (handler->GetAccessingFabricIndex() == fabricIndex)
{
ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read handler with FabricIndex: %u", fabricIndex);
ChipLogProgress(InteractionModel, "Deleting expired ReadHandler for NodeId: " ChipLogFormatX64 ", FabricIndex: %u",
ChipLogValueX64(handler->GetInitiatorNodeId()), fabricIndex);
handler->Close();
}

@@ -1452,8 +1435,9 @@ void InteractionModelEngine::OnFabricRemoved(const FabricTable & fabricTable, Fa
if (readClient->GetFabricIndex() == fabricIndex)
{
ChipLogProgress(InteractionModel, "Fabric removed, deleting obsolete read client with FabricIndex: %u", fabricIndex);
readClient->Close(CHIP_NO_ERROR, false);
RemoveReadClient(readClient);
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved
readClient->mpImEngine = nullptr;
readClient->Close(CHIP_ERROR_IM_FABRIC_DELETED, false);
}
}

7 changes: 1 addition & 6 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
@@ -144,12 +144,6 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
*/
void ShutdownAllSubscriptions();

/**
* Expire active transactions and release related objects for the given fabric index.
* This is used for releasing transactions that won't be closed when a fabric is removed.
*/
void CloseTransactionsFromFabricIndex(FabricIndex aFabricIndex);

uint32_t GetNumActiveReadHandlers() const;
uint32_t GetNumActiveReadHandlers(ReadHandler::InteractionType type) const;

@@ -290,6 +284,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
*/
uint16_t GetMinGuaranteedSubscriptionsPerFabric() const;

// virtual method from FabricTable::Delegate
void OnFabricRemoved(const FabricTable & fabricTable, FabricIndex fabricIndex) override;
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

#if CONFIG_BUILD_FOR_HOST_UNIT_TEST
2 changes: 1 addition & 1 deletion src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ ReadClient::~ReadClient()
// This won't be the case if the engine shut down before this destructor was called (in which case, mpImEngine
// will point to null)
//
if (mpImEngine && mpImEngine->InActiveReadClientList(this))
if (mpImEngine)
{
mpImEngine->RemoveReadClient(this);
}
Original file line number Diff line number Diff line change
@@ -383,11 +383,7 @@ class OpCredsFabricTableDelegate : public chip::FabricTable::Delegate
}

// Gets called when a fabric is added/updated, but not necessarily committed to storage
void OnFabricUpdated(const FabricTable & fabricTable, FabricIndex fabricIndex) override
{
ChipLogProgress(InteractionModel, "OnFabricUpdated debugging from opertonal-credential!!!!!!!");
NotifyFabricTableChanged();
}
void OnFabricUpdated(const FabricTable & fabricTable, FabricIndex fabricIndex) override { NotifyFabricTableChanged(); }

// Gets called when a fabric in FabricTable is persisted to storage
void OnFabricCommitted(const FabricTable & fabricTable, FabricIndex fabricIndex) override
14 changes: 5 additions & 9 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
@@ -3686,13 +3686,14 @@ void TestReadInteraction::TestSubscribeInvalidateFabric(nlTestSuite * apSuite, v
auto * engine = chip::app::InteractionModelEngine::GetInstance();
err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse);

ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
readPrepareParams.mEventPathParamsListSize = 0;
readPrepareParams.mpAttributePathParamsList = new chip::app::AttributePathParams[1];
readPrepareParams.mAttributePathParamsListSize = 1;

readPrepareParams.mpAttributePathParamsList = new chip::app::AttributePathParams[2];
readPrepareParams.mAttributePathParamsListSize = 2;
readPrepareParams.mpAttributePathParamsList[0].mEndpointId = Test::kMockEndpoint3;
readPrepareParams.mpAttributePathParamsList[0].mClusterId = Test::MockClusterId(2);
readPrepareParams.mpAttributePathParamsList[0].mAttributeId = Test::MockAttributeId(4);

readPrepareParams.mMinIntervalFloorSeconds = 0;
readPrepareParams.mMaxIntervalCeilingSeconds = 0;
@@ -3709,11 +3710,6 @@ void TestReadInteraction::TestSubscribeInvalidateFabric(nlTestSuite * apSuite, v
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(apSuite, delegate.mGotReport);

// We have 29 attributes in our mock attribute storage. And we subscribed twice.
// And attribute 3/2/4 is a list with 6 elements and list chunking is applied to it, thus we should receive ( 29 + 6 ) * 2 =
// 70 attribute data in total.
NL_TEST_ASSERT(apSuite, delegate.mNumAttributeResponse == 70);
NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1);
NL_TEST_ASSERT(apSuite, engine->ActiveHandlerAt(0) != nullptr);
delegate.mpReadHandler = engine->ActiveHandlerAt(0);
3 changes: 3 additions & 0 deletions src/lib/core/CHIPError.cpp
Original file line number Diff line number Diff line change
@@ -731,6 +731,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
case CHIP_ERROR_MISSING_URI_SEPARATOR.AsInteger():
desc = "The URI separator is missing";
break;
case CHIP_ERROR_IM_FABRIC_DELETED.AsInteger():
desc = "The fabric is deleted, and the corresponding IM resources are released";
break;
}
#endif // !CHIP_CONFIG_SHORT_ERROR_STR

8 changes: 8 additions & 0 deletions src/lib/core/CHIPError.h
Original file line number Diff line number Diff line change
@@ -2428,6 +2428,14 @@ using CHIP_ERROR = ::chip::ChipError;
*/
#define CHIP_ERROR_MISSING_URI_SEPARATOR CHIP_CORE_ERROR(0xe0)

/**
* @def CHIP_ERROR_IM_FABRIC_DELETED
*
* @brief
* The fabric is deleted, and the corresponding IM resources are released
*/
#define CHIP_ERROR_IM_FABRIC_DELETED CHIP_CORE_ERROR(0xe1)
yunhanw-google marked this conversation as resolved.
Show resolved Hide resolved

// clang-format on

// !!!!! IMPORTANT !!!!! If you add new CHIP errors, please update the translation