| Message ID | 20201025162730.47247-1-marijns95@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ] audio/a2dp: a2dp_channel should have a refcount on avdtp session | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=370053 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS --- Regards, Linux Bluetooth
Hi Marijn, On Sun, Oct 25, 2020 at 9:27 AM Marijn Suijten <marijns95@gmail.com> wrote: > > a2dp_channel keeps a reference to an avdtp session without incrementing > its refcount. Not only does this appear wrong, it causes unexpected > disconnections when the remote SEP responds with rejections. > > During testing with an audio application disconnections are observed > when a codec config change through MediaEndpoint1.SetConfiguration > fails. As soon as BlueZ receives this failure from the peer the > corresponding a2dp_setup object is cleaned up which holds the last > refcount to an avdtp session, in turn starting the disconnect process. > An eventual open sink/source and transport have already closed by that > time and released their refcounts. > > Adding refcounting semantics around a2dp_channel resolves the > disconnections and allows future calls on MediaEndpoint1 to safely > access the sesion stored within this channel. > --- > profiles/audio/a2dp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > index cc4866b5b..0eac0135f 100644 > --- a/profiles/audio/a2dp.c > +++ b/profiles/audio/a2dp.c > @@ -1507,6 +1507,9 @@ static void channel_free(void *data) > > avdtp_remove_state_cb(chan->state_id); > > + if (chan->session) > + avdtp_unref(chan->session); > + > queue_destroy(chan->seps, remove_remote_sep); > free(chan->last_used); > g_free(chan); > @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, > break; > case AVDTP_SESSION_STATE_CONNECTED: > if (!chan->session) > - chan->session = session; > + chan->session = avdtp_ref(session); Afaik this was done on purpose since we only need a weak reference as taking a reference would prevent the session to be disconnected when there is no setup in place, so I pretty sure this will cause regressions, instead we should probably add a reference when reconfiguring is in place and have a grace period for switching to another codec. > load_remote_seps(chan); > break; > } > @@ -2145,6 +2148,7 @@ found: > channel_remove(chan); > return NULL; > } > + avdtp_ref(chan->session); > > return avdtp_ref(chan->session); > } > @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > error("Unable to create AVDTP session"); > goto fail; > } > + avdtp_ref(chan->session); > } > > g_io_channel_unref(chan->io); > -- > 2.29.1 > > Marijn Suijten
Hi Luiz, On Mon, 26 Oct 2020 at 21:15, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Marijn, > > On Sun, Oct 25, 2020 at 9:27 AM Marijn Suijten <marijns95@gmail.com> wrote: > > > > a2dp_channel keeps a reference to an avdtp session without incrementing > > its refcount. Not only does this appear wrong, it causes unexpected > > disconnections when the remote SEP responds with rejections. > > > > During testing with an audio application disconnections are observed > > when a codec config change through MediaEndpoint1.SetConfiguration > > fails. As soon as BlueZ receives this failure from the peer the > > corresponding a2dp_setup object is cleaned up which holds the last > > refcount to an avdtp session, in turn starting the disconnect process. > > An eventual open sink/source and transport have already closed by that > > time and released their refcounts. > > > > Adding refcounting semantics around a2dp_channel resolves the > > disconnections and allows future calls on MediaEndpoint1 to safely > > access the sesion stored within this channel. > > --- > > profiles/audio/a2dp.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > index cc4866b5b..0eac0135f 100644 > > --- a/profiles/audio/a2dp.c > > +++ b/profiles/audio/a2dp.c > > @@ -1507,6 +1507,9 @@ static void channel_free(void *data) > > > > avdtp_remove_state_cb(chan->state_id); > > > > + if (chan->session) > > + avdtp_unref(chan->session); > > + > > queue_destroy(chan->seps, remove_remote_sep); > > free(chan->last_used); > > g_free(chan); > > @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, > > break; > > case AVDTP_SESSION_STATE_CONNECTED: > > if (!chan->session) > > - chan->session = session; > > + chan->session = avdtp_ref(session); > > Afaik this was done on purpose since we only need a weak reference as > taking a reference would prevent the session to be disconnected when > there is no setup in place, so I pretty sure this will cause > regressions, instead we should probably add a reference when > reconfiguring is in place and have a grace period for switching to > another codec. Allright, so it's either an in-progress setup or ongoing transport keeping the session alive, nothing else? I guess this is as simple as setting a higher dc_timeout when calling set_disconnect_timer in avdtp_unref? > > > load_remote_seps(chan); > > break; > > } > > @@ -2145,6 +2148,7 @@ found: > > channel_remove(chan); > > return NULL; > > } > > + avdtp_ref(chan->session); > > > > return avdtp_ref(chan->session); > > } > > @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > > error("Unable to create AVDTP session"); > > goto fail; > > } > > + avdtp_ref(chan->session); > > } > > > > g_io_channel_unref(chan->io); > > -- > > 2.29.1 > > > > Marijn Suijten > > > > -- > Luiz Augusto von Dentz Marijn Suijten
Hi Marijn, On Mon, Oct 26, 2020 at 1:22 PM Marijn Suijten <marijns95@gmail.com> wrote: > > Hi Luiz, > > On Mon, 26 Oct 2020 at 21:15, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Marijn, > > > > On Sun, Oct 25, 2020 at 9:27 AM Marijn Suijten <marijns95@gmail.com> wrote: > > > > > > a2dp_channel keeps a reference to an avdtp session without incrementing > > > its refcount. Not only does this appear wrong, it causes unexpected > > > disconnections when the remote SEP responds with rejections. > > > > > > During testing with an audio application disconnections are observed > > > when a codec config change through MediaEndpoint1.SetConfiguration > > > fails. As soon as BlueZ receives this failure from the peer the > > > corresponding a2dp_setup object is cleaned up which holds the last > > > refcount to an avdtp session, in turn starting the disconnect process. > > > An eventual open sink/source and transport have already closed by that > > > time and released their refcounts. > > > > > > Adding refcounting semantics around a2dp_channel resolves the > > > disconnections and allows future calls on MediaEndpoint1 to safely > > > access the sesion stored within this channel. > > > --- > > > profiles/audio/a2dp.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > > index cc4866b5b..0eac0135f 100644 > > > --- a/profiles/audio/a2dp.c > > > +++ b/profiles/audio/a2dp.c > > > @@ -1507,6 +1507,9 @@ static void channel_free(void *data) > > > > > > avdtp_remove_state_cb(chan->state_id); > > > > > > + if (chan->session) > > > + avdtp_unref(chan->session); > > > + > > > queue_destroy(chan->seps, remove_remote_sep); > > > free(chan->last_used); > > > g_free(chan); > > > @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, > > > break; > > > case AVDTP_SESSION_STATE_CONNECTED: > > > if (!chan->session) > > > - chan->session = session; > > > + chan->session = avdtp_ref(session); > > > > Afaik this was done on purpose since we only need a weak reference as > > taking a reference would prevent the session to be disconnected when > > there is no setup in place, so I pretty sure this will cause > > regressions, instead we should probably add a reference when > > reconfiguring is in place and have a grace period for switching to > > another codec. > > Allright, so it's either an in-progress setup or ongoing transport > keeping the session alive, nothing else? I guess this is as simple as > setting a higher dc_timeout when calling set_disconnect_timer in > avdtp_unref? Yep, actually now that you mentioned the dc_timeout that should have prevented us to disconnect immediately when reconfiguring as we should have been in AVDTP_SESSION_STATE_CONNECTED state but I think the problem is that we use avdtp_close to reconfigure which does set the dc_timeout to 0 since that is normally used for cleaning up the session, so I think we may need to restore the dc_timeout to DISCONNECT_TIMEOUT if we attempt to set a configuration: diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c index ae93fb26f..97b4d1b44 100644 --- a/profiles/audio/avdtp.c +++ b/profiles/audio/avdtp.c @@ -3498,6 +3498,7 @@ int avdtp_set_configuration(struct avdtp *session, session->streams = g_slist_append(session->streams, new_stream); if (stream) *stream = new_stream; + session->dc_timeout = DISCONNECT_TIMEOUT; } g_free(req); > > > > > load_remote_seps(chan); > > > break; > > > } > > > @@ -2145,6 +2148,7 @@ found: > > > channel_remove(chan); > > > return NULL; > > > } > > > + avdtp_ref(chan->session); > > > > > > return avdtp_ref(chan->session); > > > } > > > @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > > > error("Unable to create AVDTP session"); > > > goto fail; > > > } > > > + avdtp_ref(chan->session); > > > } > > > > > > g_io_channel_unref(chan->io); > > > -- > > > 2.29.1 > > > > > > Marijn Suijten > > > > > > > > -- > > Luiz Augusto von Dentz > > Marijn Suijten
Hi Luiz, On Mon, 26 Oct 2020 at 21:39, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Marijn, > > On Mon, Oct 26, 2020 at 1:22 PM Marijn Suijten <marijns95@gmail.com> wrote: > > > > Hi Luiz, > > > > On Mon, 26 Oct 2020 at 21:15, Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > Hi Marijn, > > > > > > On Sun, Oct 25, 2020 at 9:27 AM Marijn Suijten <marijns95@gmail.com> wrote: > > > > > > > > a2dp_channel keeps a reference to an avdtp session without incrementing > > > > its refcount. Not only does this appear wrong, it causes unexpected > > > > disconnections when the remote SEP responds with rejections. > > > > > > > > During testing with an audio application disconnections are observed > > > > when a codec config change through MediaEndpoint1.SetConfiguration > > > > fails. As soon as BlueZ receives this failure from the peer the > > > > corresponding a2dp_setup object is cleaned up which holds the last > > > > refcount to an avdtp session, in turn starting the disconnect process. > > > > An eventual open sink/source and transport have already closed by that > > > > time and released their refcounts. > > > > > > > > Adding refcounting semantics around a2dp_channel resolves the > > > > disconnections and allows future calls on MediaEndpoint1 to safely > > > > access the sesion stored within this channel. > > > > --- > > > > profiles/audio/a2dp.c | 7 ++++++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > > > index cc4866b5b..0eac0135f 100644 > > > > --- a/profiles/audio/a2dp.c > > > > +++ b/profiles/audio/a2dp.c > > > > @@ -1507,6 +1507,9 @@ static void channel_free(void *data) > > > > > > > > avdtp_remove_state_cb(chan->state_id); > > > > > > > > + if (chan->session) > > > > + avdtp_unref(chan->session); > > > > + > > > > queue_destroy(chan->seps, remove_remote_sep); > > > > free(chan->last_used); > > > > g_free(chan); > > > > @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, > > > > break; > > > > case AVDTP_SESSION_STATE_CONNECTED: > > > > if (!chan->session) > > > > - chan->session = session; > > > > + chan->session = avdtp_ref(session); > > > > > > Afaik this was done on purpose since we only need a weak reference as > > > taking a reference would prevent the session to be disconnected when > > > there is no setup in place, so I pretty sure this will cause > > > regressions, instead we should probably add a reference when > > > reconfiguring is in place and have a grace period for switching to > > > another codec. > > > > Allright, so it's either an in-progress setup or ongoing transport > > keeping the session alive, nothing else? I guess this is as simple as > > setting a higher dc_timeout when calling set_disconnect_timer in > > avdtp_unref? > > Yep, actually now that you mentioned the dc_timeout that should have > prevented us to disconnect immediately when reconfiguring as we should > have been in AVDTP_SESSION_STATE_CONNECTED state but I think the > problem is that we use avdtp_close to reconfigure which does set the > dc_timeout to 0 Yep a2dp_reconfig calls that functionand sets it to 0. > since that is normally used for cleaning up the > session, so I think we may need to restore the dc_timeout to > DISCONNECT_TIMEOUT if we attempt to set a configuration: My thinking as well, assuming DISCONNECT_TIMEOUT is enough (1s). > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c > index ae93fb26f..97b4d1b44 100644 > --- a/profiles/audio/avdtp.c > +++ b/profiles/audio/avdtp.c > @@ -3498,6 +3498,7 @@ int avdtp_set_configuration(struct avdtp *session, > session->streams = g_slist_append(session->streams, new_stream); > if (stream) > *stream = new_stream; > + session->dc_timeout = DISCONNECT_TIMEOUT; > } > > g_free(req); > > Thanks, works like a charm! profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=1 profiles/audio/avdtp.c:avdtp_set_configuration() 0x5565f10db6f0: int_seid=8, acp_seid=1 profiles/audio/avdtp.c:session_cb() profiles/audio/avdtp.c:avdtp_parse_rej() SET_CONFIGURATION request rejected: Configuration not supported (41) profiles/audio/a2dp.c:setconf_cfm() Source 0x5565f10fc620: Set_Configuration_Cfm profiles/audio/a2dp.c:setup_ref() 0x5565f110d540: ref=2 profiles/audio/a2dp.c:setup_unref() 0x5565f110d540: ref=1 profiles/audio/a2dp.c:setup_unref() 0x5565f110d540: ref=0 profiles/audio/a2dp.c:setup_free() 0x5565f110d540 profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=0 profiles/audio/avdtp.c:avdtp_ref() 0x5565f10db6f0: ref=1 profiles/audio/avdtp.c:set_disconnect_timer() timeout 1 profiles/audio/avdtp.c:avdtp_ref() 0x5565f10db6f0: ref=2 profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=1 profiles/audio/a2dp.c:setup_ref() 0x5565f110d540: ref=1 profiles/audio/avdtp.c:avdtp_set_configuration() 0x5565f10db6f0: int_seid=8, acp_seid=7 profiles/audio/avdtp.c:session_cb() profiles/audio/avdtp.c:avdtp_parse_resp() SET_CONFIGURATION request succeeded profiles/audio/a2dp.c:setconf_cfm() Source 0x5565f10fc620: Set_Configuration_Cfm Since you suggested this change I guess it's up to you to write a commit around it and apply it or submit it for additional review, or should I do it and combine it with the original message for a v2? > > > > > > > load_remote_seps(chan); > > > > break; > > > > } > > > > @@ -2145,6 +2148,7 @@ found: > > > > channel_remove(chan); > > > > return NULL; > > > > } > > > > + avdtp_ref(chan->session); > > > > > > > > return avdtp_ref(chan->session); > > > > } > > > > @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > > > > error("Unable to create AVDTP session"); > > > > goto fail; > > > > } > > > > + avdtp_ref(chan->session); > > > > } > > > > > > > > g_io_channel_unref(chan->io); > > > > -- > > > > 2.29.1 > > > > > > > > Marijn Suijten > > > > > > > > > > > > -- > > > Luiz Augusto von Dentz > > > > Marijn Suijten > > > > -- > Luiz Augusto von Dentz Marijn Suijten
Hi Marijn, On Mon, Oct 26, 2020 at 2:37 PM Marijn Suijten <marijns95@gmail.com> wrote: > > Hi Luiz, > > On Mon, 26 Oct 2020 at 21:39, Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi Marijn, > > > > On Mon, Oct 26, 2020 at 1:22 PM Marijn Suijten <marijns95@gmail.com> wrote: > > > > > > Hi Luiz, > > > > > > On Mon, 26 Oct 2020 at 21:15, Luiz Augusto von Dentz > > > <luiz.dentz@gmail.com> wrote: > > > > > > > > Hi Marijn, > > > > > > > > On Sun, Oct 25, 2020 at 9:27 AM Marijn Suijten <marijns95@gmail.com> wrote: > > > > > > > > > > a2dp_channel keeps a reference to an avdtp session without incrementing > > > > > its refcount. Not only does this appear wrong, it causes unexpected > > > > > disconnections when the remote SEP responds with rejections. > > > > > > > > > > During testing with an audio application disconnections are observed > > > > > when a codec config change through MediaEndpoint1.SetConfiguration > > > > > fails. As soon as BlueZ receives this failure from the peer the > > > > > corresponding a2dp_setup object is cleaned up which holds the last > > > > > refcount to an avdtp session, in turn starting the disconnect process. > > > > > An eventual open sink/source and transport have already closed by that > > > > > time and released their refcounts. > > > > > > > > > > Adding refcounting semantics around a2dp_channel resolves the > > > > > disconnections and allows future calls on MediaEndpoint1 to safely > > > > > access the sesion stored within this channel. > > > > > --- > > > > > profiles/audio/a2dp.c | 7 ++++++- > > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > > > > index cc4866b5b..0eac0135f 100644 > > > > > --- a/profiles/audio/a2dp.c > > > > > +++ b/profiles/audio/a2dp.c > > > > > @@ -1507,6 +1507,9 @@ static void channel_free(void *data) > > > > > > > > > > avdtp_remove_state_cb(chan->state_id); > > > > > > > > > > + if (chan->session) > > > > > + avdtp_unref(chan->session); > > > > > + > > > > > queue_destroy(chan->seps, remove_remote_sep); > > > > > free(chan->last_used); > > > > > g_free(chan); > > > > > @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, > > > > > break; > > > > > case AVDTP_SESSION_STATE_CONNECTED: > > > > > if (!chan->session) > > > > > - chan->session = session; > > > > > + chan->session = avdtp_ref(session); > > > > > > > > Afaik this was done on purpose since we only need a weak reference as > > > > taking a reference would prevent the session to be disconnected when > > > > there is no setup in place, so I pretty sure this will cause > > > > regressions, instead we should probably add a reference when > > > > reconfiguring is in place and have a grace period for switching to > > > > another codec. > > > > > > Allright, so it's either an in-progress setup or ongoing transport > > > keeping the session alive, nothing else? I guess this is as simple as > > > setting a higher dc_timeout when calling set_disconnect_timer in > > > avdtp_unref? > > > > Yep, actually now that you mentioned the dc_timeout that should have > > prevented us to disconnect immediately when reconfiguring as we should > > have been in AVDTP_SESSION_STATE_CONNECTED state but I think the > > problem is that we use avdtp_close to reconfigure which does set the > > dc_timeout to 0 > > Yep a2dp_reconfig calls that functionand sets it to 0. > > > since that is normally used for cleaning up the > > session, so I think we may need to restore the dc_timeout to > > DISCONNECT_TIMEOUT if we attempt to set a configuration: > > My thinking as well, assuming DISCONNECT_TIMEOUT is enough (1s). > > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c > > index ae93fb26f..97b4d1b44 100644 > > --- a/profiles/audio/avdtp.c > > +++ b/profiles/audio/avdtp.c > > @@ -3498,6 +3498,7 @@ int avdtp_set_configuration(struct avdtp *session, > > session->streams = g_slist_append(session->streams, new_stream); > > if (stream) > > *stream = new_stream; > > + session->dc_timeout = DISCONNECT_TIMEOUT; > > } > > > > g_free(req); > > > > > > Thanks, works like a charm! > > profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=1 > profiles/audio/avdtp.c:avdtp_set_configuration() 0x5565f10db6f0: > int_seid=8, acp_seid=1 > profiles/audio/avdtp.c:session_cb() > profiles/audio/avdtp.c:avdtp_parse_rej() SET_CONFIGURATION request > rejected: Configuration not supported (41) > profiles/audio/a2dp.c:setconf_cfm() Source 0x5565f10fc620: > Set_Configuration_Cfm > profiles/audio/a2dp.c:setup_ref() 0x5565f110d540: ref=2 > profiles/audio/a2dp.c:setup_unref() 0x5565f110d540: ref=1 > profiles/audio/a2dp.c:setup_unref() 0x5565f110d540: ref=0 > profiles/audio/a2dp.c:setup_free() 0x5565f110d540 > profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=0 > profiles/audio/avdtp.c:avdtp_ref() 0x5565f10db6f0: ref=1 > profiles/audio/avdtp.c:set_disconnect_timer() timeout 1 > profiles/audio/avdtp.c:avdtp_ref() 0x5565f10db6f0: ref=2 > profiles/audio/avdtp.c:avdtp_unref() 0x5565f10db6f0: ref=1 > profiles/audio/a2dp.c:setup_ref() 0x5565f110d540: ref=1 > profiles/audio/avdtp.c:avdtp_set_configuration() 0x5565f10db6f0: > int_seid=8, acp_seid=7 > profiles/audio/avdtp.c:session_cb() > profiles/audio/avdtp.c:avdtp_parse_resp() SET_CONFIGURATION > request succeeded > profiles/audio/a2dp.c:setconf_cfm() Source 0x5565f10fc620: > Set_Configuration_Cfm > > Since you suggested this change I guess it's up to you to write a > commit around it and apply it or submit it for additional review, or > should I do it and combine it with the original message for a v2? I went ahead and applied it myself, thanks for confirming it works properly. > > > > > > > > > load_remote_seps(chan); > > > > > break; > > > > > } > > > > > @@ -2145,6 +2148,7 @@ found: > > > > > channel_remove(chan); > > > > > return NULL; > > > > > } > > > > > + avdtp_ref(chan->session); > > > > > > > > > > return avdtp_ref(chan->session); > > > > > } > > > > > @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > > > > > error("Unable to create AVDTP session"); > > > > > goto fail; > > > > > } > > > > > + avdtp_ref(chan->session); > > > > > } > > > > > > > > > > g_io_channel_unref(chan->io); > > > > > -- > > > > > 2.29.1 > > > > > > > > > > Marijn Suijten > > > > > > > > > > > > > > > > -- > > > > Luiz Augusto von Dentz > > > > > > Marijn Suijten > > > > > > > > -- > > Luiz Augusto von Dentz > > Marijn Suijten
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c index cc4866b5b..0eac0135f 100644 --- a/profiles/audio/a2dp.c +++ b/profiles/audio/a2dp.c @@ -1507,6 +1507,9 @@ static void channel_free(void *data) avdtp_remove_state_cb(chan->state_id); + if (chan->session) + avdtp_unref(chan->session); + queue_destroy(chan->seps, remove_remote_sep); free(chan->last_used); g_free(chan); @@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session, break; case AVDTP_SESSION_STATE_CONNECTED: if (!chan->session) - chan->session = session; + chan->session = avdtp_ref(session); load_remote_seps(chan); break; } @@ -2145,6 +2148,7 @@ found: channel_remove(chan); return NULL; } + avdtp_ref(chan->session); return avdtp_ref(chan->session); } @@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) error("Unable to create AVDTP session"); goto fail; } + avdtp_ref(chan->session); } g_io_channel_unref(chan->io);