[Darwin] The objc subscription callback bridge is leaked for single a…

…ttribute subscriptions

.github/workflows/darwin.yaml

@@ -26,7 +26,7 @@ concurrency:
 jobs:
     darwin:
         name: Build Darwin
-        timeout-minutes: 120
+        timeout-minutes: 210
 
         if: github.actor != 'restyled-io[bot]'
         runs-on: macos-latest
@@ -64,15 +64,15 @@ jobs:
                       .environment/gn_out/.ninja_log
                       .environment/pigweed-venv/*.log
             - name: Run iOS Build Debug
-              timeout-minutes: 30
+              timeout-minutes: 50
               working-directory: src/darwin/Framework
               # For now disable unguarded-availability-new warnings because we
               # internally use APIs that we are annotating as only available on
               # new enough versions.  Maybe we should change out deployment
               # target versions instead?
               run: xcodebuild -target "Matter" -sdk iphoneos OTHER_CFLAGS='${inherited} -Wno-unguarded-availability-new'
             - name: Run iOS Build Release
-              timeout-minutes: 30
+              timeout-minutes: 50
               working-directory: src/darwin/Framework
               # For now disable unguarded-availability-new warnings because we
               # internally use APIs that we are annotating as only available on
@@ -86,7 +86,7 @@ jobs:
               run: defaults delete com.apple.dt.xctest.tool
               continue-on-error: true
             - name: Run macOS Build
-              timeout-minutes: 40
+              timeout-minutes: 70
               # Enable -Werror by hand here, because the Xcode config can't
               # enable it for various reasons.  Keep whatever Xcode settings
               # for OTHER_CFLAGS exist by using ${inherited}.

src/controller/CHIPCluster.h

@@ -52,6 +52,7 @@ using ReadResponseFailureCallback     = void (*)(void * context, CHIP_ERROR err)
 using ReadDoneCallback                = void (*)(void * context);
 using SubscriptionEstablishedCallback = void (*)(void * context);
 using ResubscriptionAttemptCallback   = void (*)(void * context, CHIP_ERROR aError, uint32_t aNextResubscribeIntervalMsec);
+using SubscriptionOnDoneCallback      = std::function<void(void)>;
 
 class DLL_EXPORT ClusterBase
 {
@@ -262,12 +263,13 @@ class DLL_EXPORT ClusterBase
                        ReadResponseFailureCallback failureCb, uint16_t minIntervalFloorSeconds, uint16_t maxIntervalCeilingSeconds,
                        SubscriptionEstablishedCallback subscriptionEstablishedCb = nullptr,
                        ResubscriptionAttemptCallback resubscriptionAttemptCb = nullptr, bool aIsFabricFiltered = true,
-                       bool aKeepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional)
+                       bool aKeepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional,
+                       SubscriptionOnDoneCallback subscriptionDoneCb = nullptr)
     {
         return SubscribeAttribute<typename AttributeInfo::DecodableType, typename AttributeInfo::DecodableArgType>(
             context, AttributeInfo::GetClusterId(), AttributeInfo::GetAttributeId(), reportCb, failureCb, minIntervalFloorSeconds,
             maxIntervalCeilingSeconds, subscriptionEstablishedCb, resubscriptionAttemptCb, aIsFabricFiltered,
-            aKeepPreviousSubscriptions, aDataVersion);
+            aKeepPreviousSubscriptions, aDataVersion, subscriptionDoneCb);
     }
 
     template <typename DecodableType, typename DecodableArgType>
@@ -276,8 +278,9 @@ class DLL_EXPORT ClusterBase
                                   uint16_t minIntervalFloorSeconds, uint16_t maxIntervalCeilingSeconds,
                                   SubscriptionEstablishedCallback subscriptionEstablishedCb = nullptr,
                                   ResubscriptionAttemptCallback resubscriptionAttemptCb = nullptr, bool aIsFabricFiltered = true,
-                                  bool aKeepPreviousSubscriptions            = false,
-                                  const Optional<DataVersion> & aDataVersion = NullOptional)
+                                  bool aKeepPreviousSubscriptions               = false,
+                                  const Optional<DataVersion> & aDataVersion    = NullOptional,
+                                  SubscriptionOnDoneCallback subscriptionDoneCb = nullptr)
     {
         auto onReportCb = [context, reportCb](const app::ConcreteAttributePath & aPath, const DecodableType & aData) {
             if (reportCb != nullptr)
@@ -311,7 +314,7 @@ class DLL_EXPORT ClusterBase
         return Controller::SubscribeAttribute<DecodableType>(
             &mExchangeManager, mSession.Get().Value(), mEndpoint, clusterId, attributeId, onReportCb, onFailureCb,
             minIntervalFloorSeconds, maxIntervalCeilingSeconds, onSubscriptionEstablishedCb, onResubscriptionAttemptCb,
-            aIsFabricFiltered, aKeepPreviousSubscriptions, aDataVersion);
+            aIsFabricFiltered, aKeepPreviousSubscriptions, aDataVersion, subscriptionDoneCb);
     }
 
     /**

src/controller/ReadInteraction.h

@@ -27,6 +27,8 @@ namespace chip {
 namespace Controller {
 namespace detail {
 
+using SubscriptionOnDoneCallback = std::function<void(void)>;
+
 template <typename DecodableAttributeType>
 struct ReportAttributeParams : public app::ReadPrepareParams
 {
@@ -40,6 +42,7 @@ struct ReportAttributeParams : public app::ReadPrepareParams
         mOnSubscriptionEstablishedCb = nullptr;
     typename TypedReadAttributeCallback<DecodableAttributeType>::OnResubscriptionAttemptCallbackType mOnResubscriptionAttemptCb =
         nullptr;
+    SubscriptionOnDoneCallback mOnDoneCb         = nullptr;
     app::ReadClient::InteractionType mReportType = app::ReadClient::InteractionType::Read;
 };
 
@@ -63,7 +66,14 @@ CHIP_ERROR ReportAttribute(Messaging::ExchangeManager * exchangeMgr, EndpointId
         readParams.mpDataVersionFilterList    = dataVersionFilters.get();
         readParams.mDataVersionFilterListSize = 1;
     }
-    auto onDone = [](TypedReadAttributeCallback<DecodableAttributeType> * callback) { chip::Platform::Delete(callback); };
+    auto onDoneCb = readParams.mOnDoneCb;
+    auto onDone   = [onDoneCb](TypedReadAttributeCallback<DecodableAttributeType> * callback) {
+        if (onDoneCb)
+        {
+            onDoneCb();
+        }
+        chip::Platform::Delete(callback);
+    };
 
     auto callback = chip::Platform::MakeUnique<TypedReadAttributeCallback<DecodableAttributeType>>(
         clusterId, attributeId, readParams.mOnReportCb, readParams.mOnErrorCb, onDone, readParams.mOnSubscriptionEstablishedCb,
@@ -151,13 +161,15 @@ CHIP_ERROR SubscribeAttribute(
         nullptr,
     typename TypedReadAttributeCallback<DecodableAttributeType>::OnResubscriptionAttemptCallbackType onResubscriptionAttemptCb =
         nullptr,
-    bool fabricFiltered = true, bool keepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional)
+    bool fabricFiltered = true, bool keepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional,
+    typename detail::SubscriptionOnDoneCallback onDoneCb = nullptr)
 {
     detail::ReportAttributeParams<DecodableAttributeType> params(sessionHandle);
     params.mOnReportCb                  = onReportCb;
     params.mOnErrorCb                   = onErrorCb;
     params.mOnSubscriptionEstablishedCb = onSubscriptionEstablishedCb;
     params.mOnResubscriptionAttemptCb   = onResubscriptionAttemptCb;
+    params.mOnDoneCb                    = onDoneCb;
     params.mMinIntervalFloorSeconds     = minIntervalFloorSeconds;
     params.mMaxIntervalCeilingSeconds   = maxIntervalCeilingSeconds;
     params.mKeepSubscriptions           = keepPreviousSubscriptions;
@@ -185,12 +197,13 @@ CHIP_ERROR SubscribeAttribute(
         onSubscriptionEstablishedCb = nullptr,
     typename TypedReadAttributeCallback<typename AttributeTypeInfo::DecodableType>::OnResubscriptionAttemptCallbackType
         onResubscriptionAttemptCb = nullptr,
-    bool fabricFiltered = true, bool keepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional)
+    bool fabricFiltered = true, bool keepPreviousSubscriptions = false, const Optional<DataVersion> & aDataVersion = NullOptional,
+    typename detail::SubscriptionOnDoneCallback onDoneCb = nullptr)
 {
     return SubscribeAttribute<typename AttributeTypeInfo::DecodableType>(
         exchangeMgr, sessionHandle, endpointId, AttributeTypeInfo::GetClusterId(), AttributeTypeInfo::GetAttributeId(), onReportCb,
         onErrorCb, aMinIntervalFloorSeconds, aMaxIntervalCeilingSeconds, onSubscriptionEstablishedCb, onResubscriptionAttemptCb,
-        fabricFiltered, keepPreviousSubscriptions, aDataVersion);
+        fabricFiltered, keepPreviousSubscriptions, aDataVersion, onDoneCb);
 }
 
 namespace detail {

src/darwin/Framework/CHIP/templates/MTRBaseClusters-src.zapt

@@ -14,6 +14,7 @@
 #include <lib/support/CHIPListUtils.h>
 #include <platform/CHIPDeviceLayer.h>
 #include <type_traits>
+#include <controller/TypedReadCallback.h>
 
 using chip::Callback::Callback;
 using chip::Callback::Cancelable;
@@ -189,7 +190,7 @@ subscriptionEstablished:(SubscriptionEstablishedHandler _Nullable)subscriptionEs
     minInterval = [minInterval copy];
     maxInterval = [maxInterval copy];
     params = [params copy];
-    new MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge(self.callbackQueue,
+    __block MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge * callbackBridge = new MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge(self.callbackQueue,
       self.device,
       {{! This treats reportHandler as taking an id for the data.  This is
           not great from a type-safety perspective, of course. }}
@@ -204,9 +205,12 @@ subscriptionEstablished:(SubscriptionEstablishedHandler _Nullable)subscriptionEs
           auto failureFn = Callback<DefaultFailureCallbackType>::FromCancelable(failure);
 
           chip::Controller::{{asUpperCamelCase parent.name}}Cluster cppCluster(exchangeManager, session, self->_endpoint);
+          MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge * innerCallbackBridge = callbackBridge;
           return cppCluster.SubscribeAttribute<TypeInfo>(successFn->mContext, successFn->mCall, failureFn->mCall, [minInterval unsignedShortValue], [maxInterval unsignedShortValue], MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge::OnSubscriptionEstablished, nil,
               params == nil || params.fabricFiltered == nil || [params.fabricFiltered boolValue],
-              params != nil && params.keepPreviousSubscriptions != nil && [params.keepPreviousSubscriptions boolValue]
+              params != nil && params.keepPreviousSubscriptions != nil && [params.keepPreviousSubscriptions boolValue],
+              chip::NullOptional,
+              [innerCallbackBridge](void) { delete innerCallbackBridge; }
           );
       }, subscriptionEstablishedHandler);
 }