From 417fac2153c605fcee37b7240c32b7a9948ca3c6 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Wed, 29 May 2024 19:15:24 +0200 Subject: [PATCH] Add patch to fix race condition especially during commissioning This adds a patch which removes some obsolte callback handling. This especially fixes race condition when calling the Python Device Controller from multiple Threads. Commands with callbacks (e.g. commissioning or opening the commissioning window) have a high likelyhood to get released early when other functions of the Python Device Controller were called simultaniously. Note that this doesn't make the Python Device Controller fully reentrant: All calls which have callbacks still share a single event object. This fixes merly some unnecessary non-reentrancy. --- ...on-Remove-obsolete-callback-handling.patch | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 0007-Python-Remove-obsolete-callback-handling.patch diff --git a/0007-Python-Remove-obsolete-callback-handling.patch b/0007-Python-Remove-obsolete-callback-handling.patch new file mode 100644 index 0000000..b8cb9c4 --- /dev/null +++ b/0007-Python-Remove-obsolete-callback-handling.patch @@ -0,0 +1,75 @@ +From 20f1c72293991ad01043660b777e53be0992bae5 Mon Sep 17 00:00:00 2001 +Message-ID: <20f1c72293991ad01043660b777e53be0992bae5.1717003814.git.stefan@agner.ch> +From: Stefan Agner +Date: Wed, 29 May 2024 19:05:43 +0200 +Subject: [PATCH] [Python] Remove obsolete callback handling + +The Call() function currently still has some callback handling code +the completeEvent and callbackRes variables. These are only used when +callbacks are in play, like pychip_DeviceController_Commission or +pychip_DeviceController_OpenCommissioningWindow. When calling these +functions CallAsyncWithCompleteCallback() needs to be used (and is +beeing used in all cases). + +In practice, on single threaded applications this is not a problem. +However, when calling the SDK from multiple threads, then another Call() +Might accidentally release a call to CallAsyncWithCompleteCallback() +early. +--- + src/controller/python/chip/ChipStack.py | 19 ------------------- + 1 file changed, 19 deletions(-) + +diff --git a/src/controller/python/chip/ChipStack.py b/src/controller/python/chip/ChipStack.py +index 6df7e41de4..a8f07941a2 100644 +--- a/src/controller/python/chip/ChipStack.py ++++ b/src/controller/python/chip/ChipStack.py +@@ -164,9 +164,6 @@ class AsyncCallableHandle: + return self._res + + +-_CompleteFunct = CFUNCTYPE(None, c_void_p, c_void_p) +-_ErrorFunct = CFUNCTYPE(None, c_void_p, c_void_p, +- c_ulong, POINTER(DeviceStatusStruct)) + _LogMessageFunct = CFUNCTYPE( + None, c_int64, c_int64, c_char_p, c_uint8, c_char_p) + _ChipThreadTaskRunnerFunct = CFUNCTYPE(None, py_object) +@@ -241,21 +238,11 @@ class ChipStack(object): + self.logger.addHandler(logHandler) + self.logger.setLevel(logging.DEBUG) + +- def HandleComplete(appState, reqState): +- self.callbackRes = True +- self.completeEvent.set() +- +- def HandleError(appState, reqState, err, devStatusPtr): +- self.callbackRes = self.ErrorToException(err, devStatusPtr) +- self.completeEvent.set() +- + @_ChipThreadTaskRunnerFunct + def HandleChipThreadRun(callback): + callback() + + self.cbHandleChipThreadRun = HandleChipThreadRun +- self.cbHandleComplete = _CompleteFunct(HandleComplete) +- self.cbHandleError = _ErrorFunct(HandleError) + # set by other modules(BLE) that require service by thread while thread blocks. + self.blockingCB = None + +@@ -357,14 +344,8 @@ class ChipStack(object): + This function is a wrapper of PostTaskOnChipThread, which includes some handling of application specific logics. + Calling this function on CHIP on CHIP mainloop thread will cause deadlock. + ''' +- # throw error if op in progress +- self.callbackRes = None +- self.completeEvent.clear() + with self.networkLock: + res = self.PostTaskOnChipThread(callFunct).Wait(timeoutMs) +- self.completeEvent.set() +- if res == 0 and self.callbackRes is not None: +- return self.callbackRes + return res + + def CallAsync(self, callFunct): +-- +2.45.1 +