[Python] Avoid crash on SubscriptionTransaction error

This reimplements project-chip#32257 without reformatting Python files.
agners · Oct 16, 2024 · 498da00 · 498da00
1 parent 601e620
commit 498da00
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 17 deletions.
diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py
@@ -572,8 +572,8 @@ def Shutdown(self):
 
         handle = chip.native.GetLibraryHandle()
         builtins.chipStack.Call(
-            lambda: handle.pychip_ReadClient_Abort(
-                self._readTransaction._pReadClient, self._readTransaction._pReadCallback))
+            lambda: handle.pychip_ReadClient_ShutdownSubscription(
+                self._readTransaction._pReadClient))
         self._isDone = True
 
     def __del__(self):
@@ -648,12 +648,10 @@ def __init__(self, future: Future, eventLoop, devCtrl, returnClusterObject: bool
         self._cache = AttributeCache(returnClusterObject=returnClusterObject)
         self._changedPathSet: Set[AttributePath] = set()
         self._pReadClient = None
-        self._pReadCallback = None
         self._resultError: Optional[PyChipError] = None
 
-    def SetClientObjPointers(self, pReadClient, pReadCallback):
+    def SetClientObjPointers(self, pReadClient):
         self._pReadClient = pReadClient
-        self._pReadCallback = pReadCallback
 
     def GetAllEventValues(self):
         return self._events
@@ -1095,7 +1093,6 @@ def Read(transaction: AsyncReadTransaction, device,
             eventPathsForCffi[idx] = cast(ctypes.c_char_p(path), c_void_p)
 
     readClientObj = ctypes.POINTER(c_void_p)()
-    readCallbackObj = ctypes.POINTER(c_void_p)()
 
     ctypes.pythonapi.Py_IncRef(ctypes.py_object(transaction))
     params = _ReadParams.parse(b'\x00' * _ReadParams.sizeof())
@@ -1115,7 +1112,6 @@ def Read(transaction: AsyncReadTransaction, device,
         lambda: handle.pychip_ReadClient_Read(
             ctypes.py_object(transaction),
             ctypes.byref(readClientObj),
-            ctypes.byref(readCallbackObj),
             device,
             ctypes.c_char_p(params),
             attributePathsForCffi,
@@ -1127,7 +1123,7 @@ def Read(transaction: AsyncReadTransaction, device,
             ctypes.c_size_t(0 if events is None else len(events)),
             eventNumberFilterPtr))
 
-    transaction.SetClientObjPointers(readClientObj, readCallbackObj)
+    transaction.SetClientObjPointers(readClientObj)
 
     if not res.is_success:
         ctypes.pythonapi.Py_DecRef(ctypes.py_object(transaction))

diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp
@@ -453,12 +453,20 @@ PyChipError pychip_WriteClient_WriteGroupAttributes(size_t groupIdSizeT, chip::C
     return ToPyChipError(err);
 }
 
-void pychip_ReadClient_Abort(ReadClient * apReadClient, ReadClientCallback * apCallback)
+void pychip_ReadClient_ShutdownSubscription(ReadClient * apReadClient)
 {
-    VerifyOrDie(apReadClient != nullptr);
-    VerifyOrDie(apCallback != nullptr);
+    // If apReadClient is nullptr, it means that its life cycle has ended (such as an error happend), and nothing needs to be done.
+    VerifyOrReturn(apReadClient != nullptr);
+    // If it is not SubscriptionType, this function should not be executed.
+    VerifyOrDie(apReadClient->IsSubscriptionType());
 
-    delete apCallback;
+    Optional<SubscriptionId> subscriptionId = apReadClient->GetSubscriptionId();
+    VerifyOrDie(subscriptionId.HasValue());
+
+    FabricIndex fabricIndex = apReadClient->GetFabricIndex();
+    NodeId nodeId           = apReadClient->GetPeerNodeId();
+
+    InteractionModelEngine::GetInstance()->ShutdownSubscription(ScopedNodeId(nodeId, fabricIndex), subscriptionId.Value());
 }
 
 void pychip_ReadClient_OverrideLivenessTimeout(ReadClient * pReadClient, uint32_t livenessTimeoutMs)
@@ -497,10 +505,10 @@ void pychip_ReadClient_GetSubscriptionTimeoutMs(ReadClient * pReadClient, uint32
     }
 }
 
-PyChipError pychip_ReadClient_Read(void * appContext, ReadClient ** pReadClient, ReadClientCallback ** pCallback,
-                                   DeviceProxy * device, uint8_t * readParamsBuf, void ** attributePathsFromPython,
-                                   size_t numAttributePaths, void ** dataversionFiltersFromPython, size_t numDataversionFilters,
-                                   void ** eventPathsFromPython, size_t numEventPaths, uint64_t * eventNumberFilter)
+PyChipError pychip_ReadClient_Read(void * appContext, ReadClient ** pReadClient, DeviceProxy * device, uint8_t * readParamsBuf,
+                                   void ** attributePathsFromPython, size_t numAttributePaths, void ** dataversionFiltersFromPython,
+                                   size_t numDataversionFilters, void ** eventPathsFromPython, size_t numEventPaths,
+                                   uint64_t * eventNumberFilter)
 {
     CHIP_ERROR err                 = CHIP_NO_ERROR;
     PyReadAttributeParams pyParams = {};
@@ -612,7 +620,6 @@ PyChipError pychip_ReadClient_Read(void * appContext, ReadClient ** pReadClient,
     }
 
     *pReadClient = readClient.get();
-    *pCallback   = callback.get();
 
     callback->AdoptReadClient(std::move(readClient));