Skip to content

Commit

Permalink
Add patch to fix race condition especially during commissioning
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
agners committed May 29, 2024
1 parent ed9b9d3 commit 417fac2
Showing 1 changed file with 75 additions and 0 deletions.
75 changes: 75 additions & 0 deletions 0007-Python-Remove-obsolete-callback-handling.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
From 20f1c72293991ad01043660b777e53be0992bae5 Mon Sep 17 00:00:00 2001
Message-ID: <20f1c72293991ad01043660b777e53be0992bae5.1717003814.git.stefan@agner.ch>
From: Stefan Agner <stefan@agner.ch>
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

0 comments on commit 417fac2

Please sign in to comment.