Host task priority and ble_hs_hci_cmd_tx()

[kenh-sq]

Preface: My project is using esp-nimble, which is mostly at nimble_1_6_0_tag. Relative to the current state of master, there are differences in the call stack, but it looks like the issue I'm about to outline is still possible.

Should it be assumed that the NimBLE stack host task (nimble_port_run()) always needs to be a higher priority than any task that would also be using the stack?

The situation I am seeing (references modified to try to align with current state of master):

1. The NimBLE host task is handling a BLE_HCI_LE_SUBEV_ENH_CONN_COMPLETE event:
   • ble_hs_hci_evt_le_enh_conn_complete() ==>
   • ble_gap_rx_conn_complete() ==>
   • ble_gap_rd_rem_sup_feat_tx() ==>
   • ble_hs_hci_cmd_tx() ==>
     • Note that ble_hs_hci_lock(); is taken as part of executing this function.
   • ble_hs_hci_wait_for_ack() ==>
   • ble_npl_sem_pend() currently waiting for the ACK packet from the controller.
   • Task yields operation to other tasks.
2. A different task, with the same priority as the NimBLE host task, is picked up and is going to update some advertising data:
   • ble_gap_ext_adv_set_data() ==>
   • INTERRUPT! BLE controller task has an event (actually an ACK) for the host:
     • ble_transport_to_hs_evt_impl() ==>
     • ble_hs_hci_rx_evt() ==>
     • ble_hs_hci_rx_ack ==>
       • ble_hs_hci_ack gets set to a non-NULL value.
     • ble_npl_sem_release();
     • Interrupt eventually yields operation back to the previous task (the task about to update advertising data, not the host task, since they have the same priority).
   • ble_gap_ext_adv_set() ==>
   • ble_hs_hci_cmd_tx() ==>
   • Debug assert! ble_hs_hci_ack is non-NULL.

Were the host task a higher priority, execution after the interrupt would return to it first since it was waiting on that semaphore. So I did just that; setting all my tasks to be one lower than the host task and I no longer have this issue. But, it didn't sit well with me that the explicit priority differences were required.

I know this is a debug assert, and could be turned off so it doesn't occur, but most of the debug asserts are related to unexpected/unexplainable conditions. I'm hoping I've given a sufficient example of a condition which does occur, but only when task priorities are the same.

Were the assert done after the call to ble_hs_hci_lock(); then the above, should-be-allowable, condition would not throw the assert. The current task would try to take the lock, but the host task already, thus the current task yields execution. The host task could now be switched to since the ACK semaphore has been released and it would eventually release the lock and then eventually yield execution. I don't think there's a chance of deadly embrace with this change.