| Message ID | 20201026195408.2528476-1-marijns95@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [BlueZ,v2] audio/media: Destroy transport if SetConfiguration fails | 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=370879 ---Test result--- ############################## Test: CheckPatch - PASS ############################## Test: CheckGitLint - PASS ############################## Test: CheckBuild - PASS ############################## Test: MakeCheck - PASS --- Regards, Linux Bluetooth
Hi Marijn, On Mon, Oct 26, 2020 at 12:54 PM Marijn Suijten <marijns95@gmail.com> wrote: > > set_configuration creates a transport before calling SetConfiguration on > the MediaEndpoint1 DBus interface. If this DBus call fails the > transport sticks around while it should instead be cleaned up. > > When the peer retries or reconnects (in case of BlueZ which cuts the > connection due to a missing recount [1]) set_configuration finds this > old transport and returns FALSE. The peer will never succeed this call > unless it randomly decides to call clear_configuration or BlueZ is > restarted. > > [1]: https://marc.info/?l=linux-bluetooth&m=160364326629847&w=2 > --- > > Changes in v2: > - Removed incorrect statement about disconnection cause > - Store reference to transport in endpoint_request instead of retrieving > it through find_device_transport > > profiles/audio/media.c | 18 ++++++++++++++++-- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > index 74064d398..c84bbe22d 100644 > --- a/profiles/audio/media.c > +++ b/profiles/audio/media.c > @@ -71,6 +71,7 @@ struct media_adapter { > > struct endpoint_request { > struct media_endpoint *endpoint; > + struct media_transport *transport; > DBusMessage *msg; > DBusPendingCall *call; > media_endpoint_cb_t cb; > @@ -298,6 +299,15 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data) > return; > } > > + if (dbus_message_is_method_call(request->msg, > + MEDIA_ENDPOINT_INTERFACE, > + "SetConfiguration")) { > + if (request->transport == NULL) > + error("Expected to destroy transport"); > + else > + media_transport_destroy(request->transport); > + } > + > dbus_error_free(&err); > goto done; > } > @@ -337,6 +347,7 @@ done: > > static gboolean media_endpoint_async_call(DBusMessage *msg, > struct media_endpoint *endpoint, > + struct media_transport *transport, > media_endpoint_cb_t cb, > void *user_data, > GDestroyNotify destroy) > @@ -358,6 +369,7 @@ static gboolean media_endpoint_async_call(DBusMessage *msg, > NULL); > > request->endpoint = endpoint; > + request->transport = transport; > request->msg = msg; > request->cb = cb; > request->destroy = destroy; > @@ -393,7 +405,8 @@ static gboolean select_configuration(struct media_endpoint *endpoint, > &capabilities, length, > DBUS_TYPE_INVALID); > > - return media_endpoint_async_call(msg, endpoint, cb, user_data, destroy); > + return media_endpoint_async_call(msg, endpoint, NULL, > + cb, user_data, destroy); > } > > static int transport_device_cmp(gconstpointer data, gconstpointer user_data) > @@ -501,7 +514,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint, > > g_dbus_get_properties(conn, path, "org.bluez.MediaTransport1", &iter); > > - return media_endpoint_async_call(msg, endpoint, cb, user_data, destroy); > + return media_endpoint_async_call(msg, endpoint, transport, > + cb, user_data, destroy); > } > > static void release_endpoint(struct media_endpoint *endpoint) > -- > 2.29.1 Applied, thanks.
diff --git a/profiles/audio/media.c b/profiles/audio/media.c index 74064d398..c84bbe22d 100644 --- a/profiles/audio/media.c +++ b/profiles/audio/media.c @@ -71,6 +71,7 @@ struct media_adapter { struct endpoint_request { struct media_endpoint *endpoint; + struct media_transport *transport; DBusMessage *msg; DBusPendingCall *call; media_endpoint_cb_t cb; @@ -298,6 +299,15 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data) return; } + if (dbus_message_is_method_call(request->msg, + MEDIA_ENDPOINT_INTERFACE, + "SetConfiguration")) { + if (request->transport == NULL) + error("Expected to destroy transport"); + else + media_transport_destroy(request->transport); + } + dbus_error_free(&err); goto done; } @@ -337,6 +347,7 @@ done: static gboolean media_endpoint_async_call(DBusMessage *msg, struct media_endpoint *endpoint, + struct media_transport *transport, media_endpoint_cb_t cb, void *user_data, GDestroyNotify destroy) @@ -358,6 +369,7 @@ static gboolean media_endpoint_async_call(DBusMessage *msg, NULL); request->endpoint = endpoint; + request->transport = transport; request->msg = msg; request->cb = cb; request->destroy = destroy; @@ -393,7 +405,8 @@ static gboolean select_configuration(struct media_endpoint *endpoint, &capabilities, length, DBUS_TYPE_INVALID); - return media_endpoint_async_call(msg, endpoint, cb, user_data, destroy); + return media_endpoint_async_call(msg, endpoint, NULL, + cb, user_data, destroy); } static int transport_device_cmp(gconstpointer data, gconstpointer user_data) @@ -501,7 +514,8 @@ static gboolean set_configuration(struct media_endpoint *endpoint, g_dbus_get_properties(conn, path, "org.bluez.MediaTransport1", &iter); - return media_endpoint_async_call(msg, endpoint, cb, user_data, destroy); + return media_endpoint_async_call(msg, endpoint, transport, + cb, user_data, destroy); } static void release_endpoint(struct media_endpoint *endpoint)