On 16.03.2017 22:48, Tanu Kaskinen wrote: > There were two bugs in the old logic. The first one: > > If a device has two profiles, the old code would start the wait timer > when the first profile connects, but when the second profile connects, > the timer would not get stopped and the CONNECTION_CHANGED hook would > not get fired, because the code for that was inside an if block that > only gets executed when the first profile connects. As a result, > module-bluez5-device loading would always be delayed until the wait > timeout expires. > > The second bug: > > A crash was observed in device_start_waiting_for_profiles(). That > function is called whenever the connected profile count changes from 0 > to 1. The function also has an assertion that checks that the timer is > not running when the function is called. That assertion crashed in the > following scenario with a headset that supports HSP and A2DP: > > 1. First HSP gets connected. The timer is started. > > 2. Then HSP gets disconnected for some reason. The timer is still > running. > > 3. Then A2DP gets connected. device_start_waiting_for_profiles() is > called, because the connected profile count changed from 0 to 1 again. > The timer is already running, so the assertion fails. > > First I thought I'd remove the assertion from > device_start_waiting_for_profiles() and just restart the timer on the > second call, but then I figured that when the device returns to the > "everything disconnected" state in step 2, it would be better to stop > the timer. The purpose of the timer is to delay the notification of the > device becoming connected, but if the device becomes disconnected during > the waiting period, the notification doesn't make sense any more, and > therefore the timer doesn't make sense either. > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=100237 > --- > src/modules/bluetooth/bluez5-util.c | 49 ++++++++++++++++++++++++++----------- > 1 file changed, 35 insertions(+), 14 deletions(-) > > Changes in v2: > - Rewrote the patch so that it also fixes the crash bug. > Looks good to me. Regards Georg
