bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.74k stars 618 forks source link

Missing finalization of avdtp connection #581

Closed amorniroli closed 8 months ago

amorniroli commented 8 months ago

Describe the bug

AVDTP connection is never released if it'closed before stream has been established successfully.. To Reproduce

Steps to reproduce the behavior:

Just try this code snippet

uint8_t cid;
bd_addr_t address; // some address

a2dp_sink_establish_stream (address, &cid);
a2dp_sink_disconnect (cid)
a2dp_sink_establish_stream (address, &cid);  // returns ERROR_CODE_COMMAND_DISALLOWED

Expected behavior

The second call to a2dp_sink_establish_stream should return ERROR_CODE_SUCCESS

HCI Packet Logs Not needed

Environment: (please complete the following information):

Additional context My suggestion for a fix

image

mringwal commented 8 months ago

Hi Alessandro. How did you run into this?

The main question here should be if it's allowed to call avdtp_disconnect before the event for avdtp_connect() was received in the first place. If yes, we need to make avdtp_connect() more robust, if not an avdtp_connect_abort() would be needed.

Your suggestion might actually work, although I'd rather implement the 'abort' behaviour manually to make sure I didn't miss a case. Let me think about that a bit more.

mringwal commented 8 months ago

Ok, please give 041b66c a try.

amorniroli commented 8 months ago

Hi @mringwal ,

in my scenario, the issue randomly occurred when calling hci_power_control (HCI_POWER_OFF) and a2dp_sink_establish_stream had been already call but still not established.

For example:

I'll try the fix in the afternoon, thank you ver much.

mringwal commented 8 months ago

Did you then try to force disconnect by calling a2dp_sink_disconnect, and then run into this?

So, the next question is if/how to trigger avdtp finalize on power off. I would make the argument, that the establish stream request could stay pending and should get executed after the next power on.

amorniroli commented 8 months ago

Did you then try to force disconnect by calling a2dp_sink_disconnect, and then run into this?

Yes, extacly.

in my A2DP_HCI_PACKET_HANDLER, for HCI_STATE_HALTING btstack event I was trying to disconnect the A2DP connection, and then ran into the issue.

I confirm that b8bf1755110d8895952df06fbc69f4ab7788e64d seems to resolve.