[pulseaudio-discuss] [PATCH v2] bluez5-util: fix profile waiting logic

[Tanu Kaskinen]

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