address comments

src/app/ReadClient.cpp

@@ -35,7 +35,7 @@ namespace app {
 /**
  * @brief The default resubscribe policy will pick a random timeslot
  * with millisecond resolution over an ever increasing window,
- * following a fibonacci sequence up to CHIP_RESUBSCRIBE_MAX_FIBONACCI_STEP_INDEX, default is 14
+ * following a fibonacci sequence up to CHIP_RESUBSCRIBE_MAX_FIBONACCI_STEP_INDEX,
  * Average of the randomized wait time past the CHIP_RESUBSCRIBE_MAX_FIBONACCI_STEP_INDEX
  * will be around one hour.
  * When the retry count resets to 0, the sequence starts from the beginning again.
@@ -99,15 +99,20 @@ void ReadClient::ClearActiveSubscriptionState()
     MoveToState(ClientState::Idle);
 }
 
+void ReadClient::StopResubscription()
+{
+    ClearActiveSubscriptionState();
+    CancelLivenessCheckTimer();
+    CancelResubscribeTimer();
+    mpCallback.OnDeallocatePaths(std::move(mReadPrepareParams));
+}
+
 ReadClient::~ReadClient()
 {
     Abort();
 
     if (IsSubscriptionType())
     {
-        CancelLivenessCheckTimer();
-        CancelResubscribeTimer();
-        mpCallback.OnDeallocatePaths(std::move(mReadPrepareParams));
         //
         // Only remove ourselves from the engine's tracker list if we still continue to have a valid pointer to it.
         // This won't be the case if the engine shut down before this destructor was called (in which case, mpImEngine
@@ -136,13 +141,18 @@ void ReadClient::Close(CHIP_ERROR aError)
 
     if (aError != CHIP_NO_ERROR)
     {
-        if (mReadPrepareParams.mResubscribePolicy != nullptr && ResubscribeIfNeeded())
+        if (ResubscribeIfNeeded())
         {
             ClearActiveSubscriptionState();
             return;
         }
         mpCallback.OnError(this, aError);
     }
+
+    if (mReadPrepareParams.mResubscribePolicy != nullptr)
+    {
+        StopResubscription();
+    }
     mpCallback.OnDone(this);
 }
 
@@ -715,7 +725,12 @@ CHIP_ERROR ReadClient::SendAutoResubscribeRequest(ReadPrepareParams && aReadPrep
         mReadPrepareParams.mResubscribePolicy = DefaultResubscribePolicy;
     }
 
-    return SendSubscribeRequest(mReadPrepareParams);
+    CHIP_ERROR err = SendSubscribeRequest(mReadPrepareParams);
+    if (err != CHIP_NO_ERROR)
+    {
+        StopResubscription();
+    }
+    return err;
 }
 
 CHIP_ERROR ReadClient::SendSubscribeRequest(ReadPrepareParams & aReadPrepareParams)

src/app/ReadClient.h

@@ -168,7 +168,9 @@ class ReadClient : public Messaging::ExchangeDelegate
         /**
          * This function is invoked when using SendAutoResubscribeRequest, where the ReadClient was configured to auto re-subscribe and the ReadPrepareParams was
          * moved into this client for management. This will have to be free'ed appropriately by the application.
-         * OnDeallocatePaths is called in ReadClient's destructor when OnDone is called and delete ReadClient
+         * If SendAutoResubscribeRequest fails, this function will be called before it returns the failure.
+         * If SendAutoResubscribeRequest succeeds, this function will be called immediately before calling OnDone.
+         * If SendAutoResubscribeRequest is not called, this function will not be called.
          */
         virtual void OnDeallocatePaths(ReadPrepareParams && aReadPrepareParams) {}
     };
@@ -247,9 +249,10 @@ class ReadClient : public Messaging::ExchangeDelegate
     ReadClient * GetNextClient() { return mpNext; }
     void SetNextClient(ReadClient * apClient) { mpNext = apClient; }
 
-    //Like SendSubscribeRequest, but the ReadClient will automatically attempt to re-establish the subscription if
-    //we decide that the subscription has dropped.  The exact behavior of the re-establishment can be controlled
-    //by setting mResubscribePolicy in the ReadPrepareParams.  If not set, a default behavior with exponential backoff will be used.
+    // Like SendSubscribeRequest, but the ReadClient will automatically attempt to re-establish the subscription if
+    // we decide that the subscription has dropped.  The exact behavior of the re-establishment can be controlled
+    // by setting mResubscribePolicy in the ReadPrepareParams.  If not set, a default behavior with exponential backoff will be used.
+    //
     // The application has to know to
     // a) allocate a ReadPrepareParams object that will have fields mpEventPathParamsList and mpAttributePathParamsList with
     // lifetimes as long as the ReadClient itself and b) free those up later in the call to OnDeallocatePaths. Note: At a given
@@ -323,6 +326,7 @@ class ReadClient : public Messaging::ExchangeDelegate
      */
     void Close(CHIP_ERROR aError);
 
+    void StopResubscription();
     void ClearActiveSubscriptionState();
 
     Messaging::ExchangeManager * mpExchangeMgr = nullptr;

src/app/ReadPrepareParams.h

@@ -31,7 +31,7 @@ namespace app {
  * @brief Used to specify the re-subscription policy. Namely, the method is invoked and provided the number of
  * retries that have occurred so far.
  *
- * aShouldResubscribe and aNextSubscriptionIntervalMsec  are outparams indicating whether and how long into
+ * aShouldResubscribe and aNextSubscriptionIntervalMsec are outparams indicating whether and how long into
  * the future a re-subscription should happen.
  */
 typedef void (*OnResubscribePolicyCB)(uint32_t aNumCumulativeRetries, uint32_t & aNextSubscriptionIntervalMsec,

src/controller/python/chip/clusters/attribute.cpp

@@ -378,6 +378,7 @@ chip::ChipError::StorageType pychip_ReadClient_ReadAttributes(void * appContext,
             params.mMinIntervalFloorSeconds   = pyParams.minInterval;
             params.mMaxIntervalCeilingSeconds = pyParams.maxInterval;
             err                               = readClient->SendAutoResubscribeRequest(std::move(params));
+            SuccessOrExit(err);
             readPaths.release();
         }
         else
@@ -442,6 +443,7 @@ chip::ChipError::StorageType pychip_ReadClient_ReadEvents(void * appContext, Dev
             params.mMinIntervalFloorSeconds   = pyParams.minInterval;
             params.mMaxIntervalCeilingSeconds = pyParams.maxInterval;
             err                               = readClient->SendAutoResubscribeRequest(std::move(params));
+            SuccessOrExit(err);
             readPaths.release();
         }
         else