espressif / esp-nimble

A fork of NimBLE stack, for use with ESP32 and ESP-IDF
Apache License 2.0
76 stars 49 forks source link

assertion inside ble_hs_lock may be incorrect #34

Closed MMI closed 1 year ago

MMI commented 2 years ago

Inside ble_hs_lock() we have BLE_HS_DBG_ASSERT(!ble_hs_locked_by_cur_task());

This lines up with the comment about not allowing nested locks.

However, if you call ble_gap_unpair() you can run afoul of this because after locking, that function may call ble_gap_terminate() which will again lock.

Without the assertions enabled, everything works. Why? Because this particular port builds the lock on top of FreeRTOS' recursive mutex.

So maybe the assert shouldn't be there?

But that's not all... the assert is checking metadata about the lock without holding the lock. Doesn't that imply a race window (maybe too small to worry about but 🤷 )

rahult-github commented 1 year ago

In the latest nimble codebase , the code has been updated to handle this case. ble_gap_unpair now basically invokes ble_gap_terminate_with_conn and there is no such nested lock issue.