Created BindingManagerContext to count all consumers using

context and release it in the proper moment.
project-chip · Apr 20, 2022 · 3c1541f · 3c1541f
1 parent c36b77a
commit 3c1541f
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 28 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp b/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp
@@ -103,7 +103,7 @@ static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, ch
     }
 }
 
-static void BoundDeviceContextReleaseHandler(void * context)
+static void BoundDeviceContextReleaseHandler(chip::BindingManagerContext * context)
 {
     (void) context;
 }

diff --git a/examples/light-switch-app/efr32/src/binding-handler.cpp b/examples/light-switch-app/efr32/src/binding-handler.cpp
@@ -132,11 +132,13 @@ void LightSwitchChangedHandler(const EmberBindingTableEntry & binding, DevicePro
     }
 }
 
-void LightSwitchContextReleaseHandler(void * context)
+void LightSwitchContextReleaseHandler(BindingManagerContext * context)
 {
     VerifyOrReturn(context != nullptr, ChipLogError(NotSpecified, "LightSwitchContextReleaseHandler: context is null"));
-    BindingCommandData * data = static_cast<BindingCommandData *>(context);
-    Platform::Delete(data);
+
+    // Release the internal context stored in BindingManagerContext and then the BindingManagerContext itself.
+    Platform::Delete(static_cast<BindingCommandData *>(context->GetContext()));
+    Platform::Delete(context);
 }
 
 #ifdef ENABLE_CHIP_SHELL
@@ -404,8 +406,9 @@ void SwitchWorkerFunction(intptr_t context)
 {
     VerifyOrReturn(context != 0, ChipLogError(NotSpecified, "SwitchWorkerFunction - Invalid work data"));
 
-    BindingCommandData * data = reinterpret_cast<BindingCommandData *>(context);
-    BindingManager::GetInstance().NotifyBoundClusterChanged(data->localEndpointId, data->clusterId, static_cast<void *>(data));
+    BindingCommandData * data              = reinterpret_cast<BindingCommandData *>(context);
+    BindingManagerContext * bindingContext = Platform::New<BindingManagerContext>(static_cast<void *>(data));
+    BindingManager::GetInstance().NotifyBoundClusterChanged(data->localEndpointId, data->clusterId, bindingContext);
 }
 
 void BindingWorkerFunction(intptr_t context)

diff --git a/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp b/examples/light-switch-app/nrfconnect/main/BindingHandler.cpp
@@ -60,10 +60,10 @@ void BindingHandler::OnInvokeCommandFailure(DeviceProxy * aDevice, BindingData &
         // Allocate new object to make sure its life time will be appropriate.
         BindingHandler::BindingData * data = Platform::New<BindingHandler::BindingData>();
         *data                              = aBindingData;
+        BindingManagerContext * context    = Platform::New<BindingManagerContext>(static_cast<void *>(data));
 
         // Establish new CASE session and retrasmit command that was not applied.
-        error = BindingManager::GetInstance().NotifyBoundClusterChanged(aBindingData.EndpointId, aBindingData.ClusterId,
-                                                                        static_cast<void *>(data));
+        error = BindingManager::GetInstance().NotifyBoundClusterChanged(aBindingData.EndpointId, aBindingData.ClusterId, context);
     }
     else
     {
@@ -226,11 +226,13 @@ void BindingHandler::LightSwitchChangedHandler(const EmberBindingTableEntry & bi
     }
 }
 
-void BindingHandler::LightSwitchContextReleaseHandler(void * context)
+void BindingHandler::LightSwitchContextReleaseHandler(BindingManagerContext * context)
 {
     VerifyOrReturn(context != nullptr, LOG_ERR("Invalid context for Light switch context release handler"););
-    BindingData * data = static_cast<BindingData *>(context);
-    Platform::Delete(data);
+
+    // Release the internal context stored in BindingManagerContext and then the BindingManagerContext itself.
+    Platform::Delete(static_cast<BindingData *>(context->GetContext()));
+    Platform::Delete(context);
 }
 
 void BindingHandler::InitInternal(intptr_t aArg)
@@ -307,7 +309,8 @@ void BindingHandler::SwitchWorkerHandler(intptr_t aContext)
 {
     VerifyOrReturn(aContext != 0, LOG_ERR("Invalid Swich data"));
 
-    BindingData * data = reinterpret_cast<BindingData *>(aContext);
+    BindingData * data              = reinterpret_cast<BindingData *>(aContext);
+    BindingManagerContext * context = Platform::New<BindingManagerContext>(static_cast<void *>(data));
     LOG_INF("Notify Bounded Cluster | endpoint: %d cluster: %d", data->EndpointId, data->ClusterId);
-    BindingManager::GetInstance().NotifyBoundClusterChanged(data->EndpointId, data->ClusterId, static_cast<void *>(data));
+    BindingManager::GetInstance().NotifyBoundClusterChanged(data->EndpointId, data->ClusterId, context);
 }
diff --git a/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h b/examples/light-switch-app/nrfconnect/main/include/BindingHandler.h
@@ -54,7 +54,7 @@ class BindingHandler
     static void OnOffProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
     static void LevelControlProcessCommand(chip::CommandId, const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
     static void LightSwitchChangedHandler(const EmberBindingTableEntry &, chip::DeviceProxy *, void *);
-    static void LightSwitchContextReleaseHandler(void * context);
+    static void LightSwitchContextReleaseHandler(chip::BindingManagerContext * context);
     static void InitInternal(intptr_t);
 
     bool mCaseSessionRecovered = false;

diff --git a/src/app/clusters/bindings/BindingManager.cpp b/src/app/clusters/bindings/BindingManager.cpp
@@ -161,8 +161,15 @@ void BindingManager::HandleDeviceConnected(OperationalDeviceProxy * device)
         {
             fabricToRemove = entry.fabricIndex;
             nodeToRemove   = entry.nodeId;
-            mBoundDeviceChangedHandler(entry, device, pendingNotification.mContext);
-            mBoundDeviceContextReleaseHandler(pendingNotification.mContext);
+
+            BindingManagerContext * context = static_cast<BindingManagerContext *>(pendingNotification.mContext);
+            mBoundDeviceChangedHandler(entry, device, context->GetContext());
+
+            context->DecrementConsumersNumber();
+            if (context->GetConsumersNumber() == 0)
+            {
+                mBoundDeviceContextReleaseHandler(context);
+            }
         }
     }
     mPendingNotificationMap.RemoveAllEntriesForNode(fabricToRemove, nodeToRemove);
@@ -187,7 +194,13 @@ void BindingManager::HandleDeviceConnectionFailure(PeerId peerId, CHIP_ERROR err
 
         if (peerId == peer)
         {
-            mBoundDeviceContextReleaseHandler(pendingNotification.mContext);
+            BindingManagerContext * context = static_cast<BindingManagerContext *>(pendingNotification.mContext);
+
+            context->DecrementConsumersNumber();
+            if (context->GetConsumersNumber() == 0)
+            {
+                mBoundDeviceContextReleaseHandler(context);
+            }
         }
     }
 
@@ -200,14 +213,15 @@ void BindingManager::FabricRemoved(CompressedFabricId compressedFabricId, Fabric
     mInitParams.mCASESessionManager->ReleaseSessionsForFabric(compressedFabricId);
 }
 
-CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context)
+CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, BindingManagerContext * context)
 {
     VerifyOrReturnError(mInitParams.mFabricTable != nullptr, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(mBoundDeviceChangedHandler, CHIP_NO_ERROR);
     VerifyOrReturnError(mBoundDeviceContextReleaseHandler, CHIP_NO_ERROR);
 
-    CHIP_ERROR error              = CHIP_NO_ERROR;
-    bool pendingNotificationAdded = false;
+    CHIP_ERROR error = CHIP_NO_ERROR;
+
+    context->IncrementConsumersNumber();
 
     for (auto iter = BindingTable::GetInstance().begin(); iter != BindingTable::GetInstance().end(); ++iter)
     {
@@ -222,27 +236,31 @@ CHIP_ERROR BindingManager::NotifyBoundClusterChanged(EndpointId endpoint, Cluste
                 if (peerDevice != nullptr && peerDevice->IsConnected())
                 {
                     // We already have an active connection
-                    mBoundDeviceChangedHandler(*iter, peerDevice, context);
+                    mBoundDeviceChangedHandler(*iter, peerDevice, context->GetContext());
                 }
                 else
                 {
-                    mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), context);
-                    pendingNotificationAdded = true;
-                    error                    = EstablishConnection(iter->fabricIndex, iter->nodeId);
+                    mPendingNotificationMap.AddPendingNotification(iter.GetIndex(), static_cast<void *>(context));
+                    context->IncrementConsumersNumber();
+                    error = EstablishConnection(iter->fabricIndex, iter->nodeId);
                     SuccessOrExit(error == CHIP_NO_ERROR);
                 }
             }
             else if (iter->type == EMBER_MULTICAST_BINDING)
             {
-                mBoundDeviceChangedHandler(*iter, nullptr, context);
+                mBoundDeviceChangedHandler(*iter, nullptr, context->GetContext());
             }
         }
     }
 
 exit:
+    context->DecrementConsumersNumber();
+
     // Call release context handler only if any pending notification is not going to use it.
-    if (!pendingNotificationAdded)
+    if (context->GetConsumersNumber() == 0)
+    {
         mBoundDeviceContextReleaseHandler(context);
+    }
 
     return error;
 }

diff --git a/src/app/clusters/bindings/BindingManager.h b/src/app/clusters/bindings/BindingManager.h
@@ -26,6 +26,26 @@
 
 namespace chip {
 
+class BindingManagerContext
+{
+public:
+    BindingManagerContext(void * context) : mContext(context) {}
+    void * GetContext() { return mContext; };
+    uint8_t GetConsumersNumber() { return mConsumersNumber; }
+    void IncrementConsumersNumber() { mConsumersNumber++; }
+    void DecrementConsumersNumber()
+    {
+        if (mConsumersNumber > 0)
+        {
+            mConsumersNumber--;
+        }
+    }
+
+private:
+    void * mContext;
+    uint8_t mConsumersNumber = 0;
+};
+
 /**
  * Application callback function when a cluster associated with a binding changes.
  *
@@ -40,7 +60,11 @@ namespace chip {
  */
 using BoundDeviceChangedHandler = void (*)(const EmberBindingTableEntry & binding, DeviceProxy * peer_device, void * context);
 
-using BoundDeviceContextReleaseHandler = void (*)(void * context);
+/**
+ * Application callback function when a context used in NotifyBoundClusterChanged will not be needed and should be
+ * released.
+ */
+using BoundDeviceContextReleaseHandler = void (*)(BindingManagerContext * context);
 
 struct BindingManagerInitParams
 {
@@ -112,7 +136,7 @@ class BindingManager
      * be initiated. The BoundDeviceChangedHandler will be called once the session is established.
      *
      */
-    CHIP_ERROR NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, void * context);
+    CHIP_ERROR NotifyBoundClusterChanged(EndpointId endpoint, ClusterId cluster, BindingManagerContext * context);
 
     static BindingManager & GetInstance() { return sBindingManager; }
-Original file line number
+Diff line change
@@ Expand Up @@
         }
     }
-    static void BoundDeviceContextReleaseHandler(void * context)
+    static void BoundDeviceContextReleaseHandler(chip::BindingManagerContext * context)
     {
         (void) context;
     }
@@ Expand Down @@