Avoid double-delete when a Darwin subscribeAttribute subscription errors out.

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -688,9 +688,8 @@ CHIP_ERROR Encode(chip::TLV::TLVWriter & writer, chip::TLV::Tag tag) const
 // Callback bridge for MTRDataValueDictionaryCallback
 class MTRDataValueDictionaryCallbackBridge : public MTRCallbackBridge<MTRDataValueDictionaryCallback> {
 public:
-    MTRDataValueDictionaryCallbackBridge(
-        dispatch_queue_t queue, MTRDeviceResponseHandler handler, MTRActionBlock action, bool keepAlive = false)
-        : MTRCallbackBridge<MTRDataValueDictionaryCallback>(queue, handler, action, OnSuccessFn, keepAlive) {};
+    MTRDataValueDictionaryCallbackBridge(dispatch_queue_t queue, MTRDeviceResponseHandler handler, MTRActionBlock action)
+        : MTRCallbackBridge<MTRDataValueDictionaryCallback>(queue, handler, action, OnSuccessFn) {};
 
     static void OnSuccessFn(void * context, id value) { DispatchSuccess(context, value); }
 };

src/darwin/Framework/CHIP/MTRCallbackBridgeBase_internal.h

@@ -72,10 +72,9 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
      * Construct a callback bridge, which can then have DispatcLocalAction() called
      * on it.
      */
-    MTRCallbackBridge(dispatch_queue_t queue, MTRResponseHandler handler, T OnSuccessFn, bool keepAlive)
+    MTRCallbackBridge(dispatch_queue_t queue, MTRResponseHandler handler, T OnSuccessFn)
         : mQueue(queue)
         , mHandler(handler)
-        , mKeepAlive(keepAlive)
         , mSuccess(OnSuccessFn)
         , mFailure(OnFailureFn)
     {
@@ -85,11 +84,10 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
      * Construct a callback bridge, which can then have DispatchAction() called
      * on it.
      */
-    MTRCallbackBridge(dispatch_queue_t queue, MTRResponseHandler handler, MTRActionBlock action, T OnSuccessFn, bool keepAlive)
+    MTRCallbackBridge(dispatch_queue_t queue, MTRResponseHandler handler, MTRActionBlock action, T OnSuccessFn)
         : mQueue(queue)
         , mHandler(handler)
         , mAction(action)
-        , mKeepAlive(keepAlive)
         , mSuccess(OnSuccessFn)
         , mFailure(OnFailureFn)
     {
@@ -219,6 +217,26 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
     static void DispatchFailure(void * context, NSError * error) { DispatchCallbackResult(context, error, nil); }
 
 protected:
+    // OnDone and KeepAliveOnCallback really only make sense for subscription
+    // bridges, but we put them here to avoid many copies of this code in
+    // generated bits.
+    void OnDone()
+    {
+        if (!mQueue) {
+            delete this;
+            return;
+        }
+
+        // Delete ourselves async, so that any error/data reports we
+        // queued up before getting OnDone have a chance to run.
+        auto * self = this;
+        dispatch_async(mQueue, ^{
+            delete self;
+        });
+    }
+
+    void KeepAliveOnCallback() { mKeepAlive = true; }
+
     dispatch_queue_t mQueue;
 
 private:
@@ -230,15 +248,12 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
         }
 
         if (!callbackBridge->mQueue) {
-            delete callbackBridge;
+            if (!callbackBridge->mKeepAlive) {
+                delete callbackBridge;
+            }
             return;
         }
 
-        if (error) {
-            // We should delete ourselves; there will be no more callbacks.
-            callbackBridge->mKeepAlive = false;
-        }
-
         dispatch_async(callbackBridge->mQueue, ^{
             ChipLogDetail(Controller, "%s %f seconds", callbackBridge->mCookie.UTF8String,
                 -[callbackBridge->mRequestTime timeIntervalSinceNow]);
@@ -252,7 +267,7 @@ template <class T> class MTRCallbackBridge : public MTRCallbackBridgeBase {
 
     MTRResponseHandler mHandler;
     MTRActionBlock mAction;
-    bool mKeepAlive;
+    bool mKeepAlive = false;
 
     T mSuccess;
     MTRErrorCallback mFailure;

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

@@ -195,12 +195,18 @@ reportHandler:(void (^)({{asObjectiveCClass type parent.name}} * _Nullable value
           using TypeInfo = {{asUpperCamelCase parent.name}}::Attributes::{{asUpperCamelCase name}}::TypeInfo;
 
           chip::Controller::{{asUpperCamelCase parent.name}}Cluster cppCluster(exchangeManager, session, self->_endpoint);
-          return cppCluster.SubscribeAttribute<TypeInfo>(bridge, successCb, failureCb, [params.minInterval unsignedShortValue], [params.maxInterval unsignedShortValue], MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge::OnSubscriptionEstablished, nil,
+          CHIP_ERROR err = cppCluster.SubscribeAttribute<TypeInfo>(bridge, successCb, failureCb, [params.minInterval unsignedShortValue], [params.maxInterval unsignedShortValue], MTR{{>attribute_data_callback_name}}CallbackSubscriptionBridge::OnSubscriptionEstablished, nil,
               params.filterByFabric,
               !params.replaceExistingSubscriptions,
               chip::NullOptional,
-              [typedBridge](void) { delete typedBridge; }
+              [typedBridge](void) { typedBridge->OnDone(); }
           );
+          if (err == CHIP_NO_ERROR) {
+              // Now that we kicked off the subscribe, flag our callback bridge
+              // to stay alive until we get an OnDone.
+              typedBridge->KeepAliveOnCallback();
+          }
+          return err;
       }, subscriptionEstablished);
     std::move(*bridge).DispatchAction(self.device);
 }

src/darwin/Framework/CHIP/templates/partials/MTRCallbackBridge.zapt

@@ -13,12 +13,12 @@
 class MTR{{> @partial-block}}Bridge : public MTRCallbackBridge<{{>callbackType}}>
 {
 public:
-    MTR{{> @partial-block}}Bridge(dispatch_queue_t queue, ResponseHandler handler, bool keepAlive = false)
-      : MTRCallbackBridge<{{>callbackType}}>(queue, handler, OnSuccessFn, keepAlive)
+    MTR{{> @partial-block}}Bridge(dispatch_queue_t queue, ResponseHandler handler)
+      : MTRCallbackBridge<{{>callbackType}}>(queue, handler, OnSuccessFn)
       {};
 
-    MTR{{> @partial-block}}Bridge(dispatch_queue_t queue, ResponseHandler handler, MTRActionBlock action, bool keepAlive = false)
-      : MTRCallbackBridge<{{>callbackType}}>(queue, handler, action, OnSuccessFn, keepAlive)
+    MTR{{> @partial-block}}Bridge(dispatch_queue_t queue, ResponseHandler handler, MTRActionBlock action)
+      : MTRCallbackBridge<{{>callbackType}}>(queue, handler, action, OnSuccessFn)
       {};
 
     static void OnSuccessFn(void * context
@@ -43,11 +43,13 @@ class MTR{{> @partial-block}}SubscriptionBridge : public MTR{{> @partial-block}}
 {
 public:
     MTR{{> @partial-block}}SubscriptionBridge(dispatch_queue_t queue, ResponseHandler handler, MTRActionBlock action, MTRSubscriptionEstablishedHandler establishedHandler)
-      : MTR{{> @partial-block}}Bridge(queue, handler, action, true),
+      : MTR{{> @partial-block}}Bridge(queue, handler, action),
         mEstablishedHandler(establishedHandler)
       {}
 
     static void OnSubscriptionEstablished(void * context);
+    using MTR{{> @partial-block}}Bridge::OnDone;
+    using MTR{{> @partial-block}}Bridge::KeepAliveOnCallback;
 
 private:
     MTRSubscriptionEstablishedHandler mEstablishedHandler;
@@ -110,5 +112,6 @@ void MTR{{> @partial-block}}SubscriptionBridge::OnSubscriptionEstablished(void *
          self->mEstablishedHandler = nil;
      }
 }
+
 {{/unless}}
 {{/if}}