On Mon, 2017-03-27 at 21:20 +0200, Georg Chini wrote: > 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. Thanks for the review! Pushed. -- Tanu https://www.patreon.com/tanuk
