Clear exchange context when subscription livess timeout (project-chip…

…#10061) -- After sending out status report for unsolicited report during post-subscription, clear out exchange context. Also Clear out exchange context when liveness timeout happens. -- Introduce GetPeerNodeId for read client and remove unsafe GetExchangeContext for read client.
jimlyall-q · Oct 5, 2021 · 6d2b3a2 · 6d2b3a2
1 parent 96003a7
commit 6d2b3a2
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 8 deletions.
diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -83,6 +83,7 @@ void ReadClient::ShutdownInternal(CHIP_ERROR aError)
     mpExchangeMgr              = nullptr;
     mpExchangeCtx              = nullptr;
     mInitialReport             = true;
+    mPeerNodeId                = kUndefinedNodeId;
     MoveToState(ClientState::Uninitialized);
 }
 
@@ -176,6 +177,9 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
     err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf),
                                      Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
     SuccessOrExit(err);
+
+    mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
+
     MoveToState(ClientState::AwaitingInitialReport);
 
 exit:
@@ -325,6 +329,7 @@ CHIP_ERROR ReadClient::OnUnsolicitedReportData(Messaging::ExchangeContext * apEx
 {
     mpExchangeCtx  = apExchangeContext;
     CHIP_ERROR err = ProcessReportData(std::move(aPayload));
+    mpExchangeCtx  = nullptr;
     if (err != CHIP_NO_ERROR)
     {
         ShutdownInternal(err);
@@ -552,13 +557,15 @@ void ReadClient::CancelLivenessCheckTimer()
 void ReadClient::OnLivenessTimeoutCallback(System::Layer * apSystemLayer, void * apAppState)
 {
     ReadClient * const client = reinterpret_cast<ReadClient *>(apAppState);
-    ChipLogError(DataManagement, "Subscription Liveness timeout, shutting down");
     if (client->IsFree())
     {
         ChipLogError(DataManagement,
                      "ReadClient::OnLivenessTimeoutCallback invoked on a free client! This is a bug in CHIP stack!");
         return;
     }
+
+    ChipLogError(DataManagement, "Subscription Liveness timeout with peer node 0x%" PRIx64 ", shutting down ", client->mPeerNodeId);
+    client->mpExchangeCtx = nullptr;
     // TODO: add a more specific error here for liveness timeout failure to distinguish between other classes of timeouts (i.e
     // response timeouts).
     client->ShutdownInternal(CHIP_ERROR_TIMEOUT);
@@ -645,6 +652,8 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPreparePara
     err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeRequest, std::move(msgBuf),
                                      Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
     SuccessOrExit(err);
+
+    mPeerNodeId = aReadPrepareParams.mSessionHandle.GetPeerNodeId();
     MoveToState(ClientState::AwaitingInitialReport);
 
 exit:

diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h
@@ -93,7 +93,8 @@ class ReadClient : public Messaging::ExchangeDelegate
     CHIP_ERROR SendSubscribeRequest(ReadPrepareParams & aSubscribePrepareParams);
     CHIP_ERROR OnUnsolicitedReportData(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload);
     uint64_t GetAppIdentifier() const { return mAppIdentifier; }
-    Messaging::ExchangeContext * GetExchangeContext() const { return mpExchangeCtx; }
+
+    NodeId GetPeerNodeId() const { return mPeerNodeId; }
     bool IsReadType() { return mInteractionType == InteractionType::Read; }
     bool IsSubscriptionType() const { return mInteractionType == InteractionType::Subscribe; };
     CHIP_ERROR SendStatusResponse(CHIP_ERROR aError);
@@ -177,6 +178,7 @@ class ReadClient : public Messaging::ExchangeDelegate
     uint16_t mMinIntervalFloorSeconds          = 0;
     uint16_t mMaxIntervalCeilingSeconds        = 0;
     uint64_t mSubscriptionId                   = 0;
+    NodeId mPeerNodeId                         = kUndefinedNodeId;
     InteractionType mInteractionType           = InteractionType::Read;
 };
 

diff --git a/src/app/util/im-client-callbacks.cpp b/src/app/util/im-client-callbacks.cpp
@@ -372,7 +372,7 @@ bool IMReadReportAttributesResponseCallback(const app::ReadClient * apReadClient
     Callback::Cancelable * onSuccessCallback = nullptr;
     Callback::Cancelable * onFailureCallback = nullptr;
     app::TLVDataFilter tlvFilter             = nullptr;
-    NodeId sourceId                          = apReadClient->GetExchangeContext()->GetSecureSession().GetPeerNodeId();
+    NodeId sourceId                          = apReadClient->GetPeerNodeId();
     // In CHIPClusters.cpp, we are using sequenceNumber as application identifier.
     uint8_t sequenceNumber = static_cast<uint8_t>(apReadClient->GetAppIdentifier());
 
@@ -437,8 +437,8 @@ bool IMSubscribeResponseCallback(const chip::app::ReadClient * apSubscribeClient
     CHIP_ERROR err                           = CHIP_NO_ERROR;
     Callback::Cancelable * onSuccessCallback = nullptr;
     Callback::Cancelable * onFailureCallback = nullptr;
-    err = gCallbacks.GetResponseCallback(apSubscribeClient->GetExchangeContext()->GetSecureSession().GetPeerNodeId(),
-                                         sequenceNumber, &onSuccessCallback, &onFailureCallback);
+    err =
+        gCallbacks.GetResponseCallback(apSubscribeClient->GetPeerNodeId(), sequenceNumber, &onSuccessCallback, &onFailureCallback);
 
     if (CHIP_NO_ERROR != err)
     {

diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp
@@ -1656,7 +1656,7 @@ void DeviceControllerInteractionModelDelegate::OnReportData(const app::ReadClien
 CHIP_ERROR DeviceControllerInteractionModelDelegate::ReadError(const app::ReadClient * apReadClient, CHIP_ERROR aError)
 {
     app::ClusterInfo path;
-    path.mNodeId = apReadClient->GetExchangeContext()->GetSecureSession().GetPeerNodeId();
+    path.mNodeId = apReadClient->GetPeerNodeId();
     IMReadReportAttributesResponseCallback(apReadClient, path, nullptr, Protocols::InteractionModel::ProtocolCode::Failure);
     return CHIP_NO_ERROR;
 }

diff --git a/src/controller/python/chip/interaction_model/Delegate.cpp b/src/controller/python/chip/interaction_model/Delegate.cpp
@@ -125,8 +125,7 @@ void PythonInteractionModelDelegate::OnReportData(const app::ReadClient * apRead
         if (CHIP_NO_ERROR == err)
         {
             AttributePath path{ .endpointId = aPath.mEndpointId, .clusterId = aPath.mClusterId, .fieldId = aPath.mFieldId };
-            onReportDataFunct(apReadClient->GetExchangeContext()->GetSecureSession().GetPeerNodeId(),
-                              apReadClient->GetAppIdentifier(),
+            onReportDataFunct(apReadClient->GetPeerNodeId(), apReadClient->GetAppIdentifier(),
                               /* TODO: Use real SubscriptionId */ apReadClient->IsSubscriptionType() ? 1 : 0, &path, sizeof(path),
                               writerBuffer, writer.GetLengthWritten(), to_underlying(status));
         }