diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 7348876344b7d5..9af528e09bc057 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -95,26 +95,11 @@ void InteractionModelEngine::Shutdown() mReadHandlers.ReleaseAll(); // - // We hold weak references to ReadClient objects. The application ultimately - // actually owns them, so it's on them to eventually shut them down and free them - // up. + // Applications should be terminating their ReadClient instances first BEFORE shutting + // down the IM engine. Otherwise, the ReadClient is going to call into the engine for + // services and crash in other wierd ways. // - // However, we should null out their pointers back to us at the very least so that - // at destruction time, they won't attempt to reach back here to remove themselves - // from this list. - // - for (auto * readClient = mpActiveReadClientList; readClient != nullptr;) - { - readClient->mpImEngine = nullptr; - auto * tmpClient = readClient->GetNextClient(); - readClient->SetNextClient(nullptr); - readClient = tmpClient; - } - - // - // After that, we just null out our tracker. - // - mpActiveReadClientList = nullptr; + VerifyOrDie(mpActiveReadClientList == nullptr); for (auto & writeHandler : mWriteHandlers) { diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index c4aa3fd32fd99e..eb6c423a085656 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -2119,6 +2119,10 @@ void TestReadInteraction::TestReadShutdown(nlTestSuite * apSuite, void * apConte app::ReadClient * pClients[4]; TestContext & ctx = *static_cast(apContext); MockInteractionModelApp delegate; + CHIP_ERROR err = CHIP_NO_ERROR; + + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); // // Allocate a number of clients @@ -2129,12 +2133,10 @@ void TestReadInteraction::TestReadShutdown(nlTestSuite * apSuite, void * apConte chip::app::ReadClient::InteractionType::Subscribe); } - // - // Delete every other client to ensure we test out - // deleting clients from the list of clients tracked by the IM - // Platform::Delete(pClients[1]); Platform::Delete(pClients[3]); + Platform::Delete(pClients[2]); + Platform::Delete(pClients[0]); // // Shutdown the engine first so that we can @@ -2142,13 +2144,8 @@ void TestReadInteraction::TestReadShutdown(nlTestSuite * apSuite, void * apConte // engine->Shutdown(); - // - // Shutdown the read clients. These should - // safely destruct without causing any egregious - // harm - // - Platform::Delete(pClients[0]); - Platform::Delete(pClients[2]); + err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } void TestReadInteraction::TestSubscribeInvalidInterval(nlTestSuite * apSuite, void * apContext) diff --git a/src/controller/java/AndroidCallbacks.cpp b/src/controller/java/AndroidCallbacks.cpp index 588f47c14a9389..b3bdeeb7639540 100644 --- a/src/controller/java/AndroidCallbacks.cpp +++ b/src/controller/java/AndroidCallbacks.cpp @@ -547,7 +547,7 @@ void ReportEventCallback::OnSubscriptionEstablished(SubscriptionId aSubscription JniReferences::GetInstance().CallSubscriptionEstablished(mSubscriptionEstablishedCallbackRef); } -void ReportEventCallback::OnResubscriptionAttempt(CHIP_ERROR aTerminationCause, uint32_t aNextResubscribeIntervalMsec) +void ReportEventCallback::OnResubscriptionAttempt(ReadClient * apReadClient, CHIP_ERROR aTerminationCause) { VerifyOrReturn(mResubscriptionAttemptCallbackRef != nullptr, ChipLogError(Controller, "mResubscriptionAttemptCallbackRef is null")); @@ -555,6 +555,13 @@ void ReportEventCallback::OnResubscriptionAttempt(CHIP_ERROR aTerminationCause, CHIP_ERROR err = CHIP_NO_ERROR; JNIEnv * env = JniReferences::GetInstance().GetEnvForCurrentThread(); + err = app::ReadClient::Callback::OnResubscriptionNeeded(apReadClient, aTerminationCause); + if (err != CHIP_NO_ERROR) + { + ReportError(nullptr, ErrorStr(err), err.AsInteger()); + return; + } + jmethodID onResubscriptionAttemptMethod; err = JniReferences::GetInstance().FindMethod(env, mResubscriptionAttemptCallbackRef, "onResubscriptionAttempt", "(II)V", &onResubscriptionAttemptMethod); @@ -562,7 +569,7 @@ void ReportEventCallback::OnResubscriptionAttempt(CHIP_ERROR aTerminationCause, DeviceLayer::StackUnlock unlock; env->CallVoidMethod(mResubscriptionAttemptCallbackRef, onResubscriptionAttemptMethod, aTerminationCause.AsInteger(), - aNextResubscribeIntervalMsec); + apReadClient->ComputeTimeTillNextSubscription()); } void ReportEventCallback::ReportError(jobject attributePath, CHIP_ERROR err) diff --git a/src/controller/java/AndroidCallbacks.h b/src/controller/java/AndroidCallbacks.h index 51c38d3c2b4e1a..f4ab4c22dd6b1a 100644 --- a/src/controller/java/AndroidCallbacks.h +++ b/src/controller/java/AndroidCallbacks.h @@ -98,7 +98,7 @@ struct ReportEventCallback : public app::ReadClient::Callback void OnSubscriptionEstablished(SubscriptionId aSubscriptionId) override; - void OnResubscriptionAttempt(CHIP_ERROR aTerminationCause, uint32_t aNextResubscribeIntervalMsec) override; + void OnResubscriptionAttempt(ReadClient * apReadClient, CHIP_ERROR aTerminationCause) override; /** Report errors back to Java layer. attributePath may be nullptr for general errors. */ void ReportError(jobject eventPath, CHIP_ERROR err);