search

NordicSemiconductor / pc-ble-driver-py

Python bindings for the ble-driver library
Other
126 stars 116 forks source link

Race Conditions in ble_adapter.EvtSync #55

Open alexbarteau-temboo opened 6 years ago

alexbarteau-temboo commented 6 years ago

There are 2 race conditions as a result of the design of EvtSync. Fixes for them as well as a test proving the second race condition are in the attached file evt_sync.txt.

Race Condition on Data

First EvtSync.data is a single variable that is protected by multiple Locks and therefore is not protected against multiple simultaneous accesses by calling EvtSync.notify on separate 'evts'.

This is addressed by converting self.data to a dictionary that uses 'evts' as keys.

Callback race condition

The second issue is systemic throughout the codebase. The paradigm of calling a method that causes a message to be received then waiting for result with EvtSync.wait is a constant race condition. If the message that will be waited on is received before the EvtSync.wait() is called, then message is missed. This was first noticed in ble_adapter.authenticate, but is a very common paradigm in the code base.

This happens because the callbacks are handled in a separate thread from the initiator, and therefore there is no guarantee for continuous execution to the wait unless the call is also protected with a lock.

The solution for this is a protected_call method in EvtSync that prevents the callback from notifying until after the initiator has become to wait for the message.

Expected Test Results:

Test the "call then wait for event" paradigm used regularly in the nordic pc-ble-driver-py library
Slow func sleep 0.1:    Call then Wait: Successfully Synched! 0
Slow func sleep 0.25:   Call then Wait: Successfully Synched! 1
Slow func sleep 1:  Call then Wait: None
Slow func sleep 2:  Call then Wait: None
Test the new proposed protected call paradigm.
Slow func sleep 0.1s:   Protected Call: Successfully Synched! 4
Slow func sleep 0.25s:  Protected Call: Successfully Synched! 5
Slow func sleep 1s: Protected Call: Successfully Synched! 6
Slow func sleep 2s: Protected Call: Successfully Synched! 7

None indicates that message was missed. The first 2 tests may also fail due to the schedulers impact on race conditions. The last 4 tests should always be successful.

bihanssen commented 5 years ago

Hi, sorry for late response, and thanks for reporting. We are aware of the weaknesses in the EvtSync model. We will keep your report for reference for later improvements.

andrew-gillan commented 4 years ago

Will this issue be fixed soon? I've run into right away simply trying to do the initial service discovery.