public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb()
@ 2025-10-15 15:40 Pauli Virtanen
  2025-10-15 15:40 ` [PATCH BlueZ 2/2] transport: wait until BAP fd is writable/readable before resuming Pauli Virtanen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Pauli Virtanen @ 2025-10-15 15:40 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Pauli Virtanen

Ordering of STREAMING and iso_connect_cb() is unspecified, as
kernel/Glib do not preserve the order of these events.

STREAMING before iso_connect_cb() causes transport to fail Acquire()
since bt_bap_stream_io::connecting == true.

Fix by marking IO as connected even though the connection didn't yet
complete. The socket fd is valid, although not yet writable/readable.

Fixes: https://github.com/bluez/bluez/issues/1506
---
 profiles/audio/bap.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 2994881cc..b776511c5 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2874,6 +2874,27 @@ static void bap_state(struct bt_bap_stream *stream, uint8_t old_state,
 		}
 		break;
 	case BT_BAP_STREAM_STATE_STREAMING:
+		/* Order of STREAMING and iso_connect_cb() is nondeterministic.
+		 *
+		 * If iso_connect_cb() did not complete yet, mark IO as
+		 * connected regardless, otherwise transport fails acquiring it.
+		 * If the connect doesn't actually succeed, it is handled via
+		 * normal disconnect flow.
+		 */
+		if (setup) {
+			int fd;
+
+			if (!setup->io || !setup->cis_active)
+				break;
+			if (!bt_bap_stream_io_is_connecting(stream, &fd))
+				break;
+			if (fd != g_io_channel_unix_get_fd(setup->io))
+				break;
+
+			DBG("setup %p stream %p io not yet ready",
+								setup, stream);
+			bt_bap_stream_set_io(stream, fd);
+		}
 		break;
 	}
 }
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH BlueZ 2/2] transport: wait until BAP fd is writable/readable before resuming
  2025-10-15 15:40 [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb() Pauli Virtanen
@ 2025-10-15 15:40 ` Pauli Virtanen
  2025-10-15 17:35 ` [BlueZ,1/2] bap: tolerate inverted STREAMING and iso_connect_cb() bluez.test.bot
  2025-10-20 13:20 ` [PATCH BlueZ 1/2] " patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: Pauli Virtanen @ 2025-10-15 15:40 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Pauli Virtanen

Connection may still be pending when entering STREAMING.

Wait until the fd becomes readable/writable before continuing.
---
 profiles/audio/transport.c | 69 ++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 83e3c9791..08ed699b2 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -108,6 +108,7 @@ struct bap_transport {
 	struct bt_bap_qos	qos;
 	guint			resume_id;
 	struct iovec		*meta;
+	guint			chan_id;
 };
 
 struct media_transport_ops {
@@ -2098,10 +2099,20 @@ static guint transport_bap_suspend(struct media_transport *transport,
 	return id;
 }
 
+static void bap_clear_chan(struct bap_transport *bap)
+{
+	if (bap->chan_id) {
+		g_source_remove(bap->chan_id);
+		bap->chan_id = 0;
+	}
+}
+
 static void transport_bap_cancel(struct media_transport *transport, guint id)
 {
 	struct bap_transport *bap = transport->data;
 
+	bap_clear_chan(bap);
+
 	if (id == bap->resume_id && bap->resume_id) {
 		g_source_remove(bap->resume_id);
 		bap->resume_id = 0;
@@ -2162,6 +2173,38 @@ static void bap_metadata_changed(struct media_transport *transport)
 	}
 }
 
+static gboolean bap_transport_fd_ready(GIOChannel *chan, GIOCondition cond,
+								gpointer data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = transport->data;
+	int fd;
+	uint16_t imtu, omtu;
+	GError *err = NULL;
+
+	if (cond & (G_IO_HUP | G_IO_ERR | G_IO_NVAL)) {
+		error("Transport connection failed");
+		goto done;
+	}
+
+	if (!bt_io_get(chan, &err, BT_IO_OPT_OMTU, &omtu,
+					BT_IO_OPT_IMTU, &imtu,
+					BT_IO_OPT_INVALID)) {
+		error("%s", err->message);
+		goto done;
+	}
+
+	fd = g_io_channel_unix_get_fd(chan);
+	media_transport_set_fd(transport, fd, imtu, omtu);
+	transport_update_playing(transport, TRUE);
+
+done:
+	bap->chan_id = 0;
+	bap_resume_complete(transport);
+
+	return FALSE;
+}
+
 static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 				uint8_t new_state, void *user_data)
 {
@@ -2170,9 +2213,7 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 	struct media_owner *owner = transport->owner;
 	struct io *io;
 	GIOChannel *chan;
-	GError *err = NULL;
 	int fd;
-	uint16_t imtu, omtu;
 
 	if (bap->stream != stream)
 		return;
@@ -2209,7 +2250,6 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 			bap_update_bcast_qos(transport);
 
 		bap_metadata_changed(transport);
-		transport_update_playing(transport, TRUE);
 		break;
 	case BT_BAP_STREAM_STATE_RELEASING:
 		if (bt_bap_stream_io_dir(stream) == BT_BAP_BCAST_SINK)
@@ -2231,21 +2271,20 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 		goto done;
 	}
 
+	/* Wait for FD to become ready */
+	bap_clear_chan(bap);
+
 	chan = g_io_channel_unix_new(fd);
-
-	if (!bt_io_get(chan, &err, BT_IO_OPT_OMTU, &omtu,
-					BT_IO_OPT_IMTU, &imtu,
-					BT_IO_OPT_INVALID)) {
-		error("%s", err->message);
-		goto done;
-	}
-
+	bap->chan_id = g_io_add_watch(chan,
+				G_IO_OUT | G_IO_IN |
+				G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+				bap_transport_fd_ready, transport);
 	g_io_channel_unref(chan);
-
-	media_transport_set_fd(transport, fd, imtu, omtu);
-	transport_update_playing(transport, TRUE);
+	if (bap->chan_id)
+		return;
 
 done:
+	bap_clear_chan(bap);
 	bap_resume_complete(transport);
 }
 
@@ -2287,6 +2326,8 @@ static void transport_bap_destroy(void *data)
 {
 	struct bap_transport *bap = data;
 
+	bap_clear_chan(bap);
+
 	util_iov_free(bap->meta, 1);
 	bt_bap_state_unregister(bt_bap_stream_get_session(bap->stream),
 							bap->state_id);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [BlueZ,1/2] bap: tolerate inverted STREAMING and iso_connect_cb()
  2025-10-15 15:40 [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb() Pauli Virtanen
  2025-10-15 15:40 ` [PATCH BlueZ 2/2] transport: wait until BAP fd is writable/readable before resuming Pauli Virtanen
@ 2025-10-15 17:35 ` bluez.test.bot
  2025-10-20 13:20 ` [PATCH BlueZ 1/2] " patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2025-10-15 17:35 UTC (permalink / raw)
  To: linux-bluetooth, pav

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

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=1011959

---Test result---

Test Summary:
CheckPatch                    PENDING   0.35 seconds
GitLint                       PENDING   0.84 seconds
BuildEll                      PASS      21.40 seconds
BluezMake                     PASS      4532.07 seconds
MakeCheck                     PASS      21.63 seconds
MakeDistcheck                 PASS      196.56 seconds
CheckValgrind                 PASS      256.43 seconds
CheckSmatch                   PASS      326.57 seconds
bluezmakeextell               PASS      138.99 seconds
IncrementalBuild              PENDING   0.27 seconds
ScanBuild                     PASS      1004.61 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb()
  2025-10-15 15:40 [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb() Pauli Virtanen
  2025-10-15 15:40 ` [PATCH BlueZ 2/2] transport: wait until BAP fd is writable/readable before resuming Pauli Virtanen
  2025-10-15 17:35 ` [BlueZ,1/2] bap: tolerate inverted STREAMING and iso_connect_cb() bluez.test.bot
@ 2025-10-20 13:20 ` patchwork-bot+bluetooth
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+bluetooth @ 2025-10-20 13:20 UTC (permalink / raw)
  To: Pauli Virtanen; +Cc: linux-bluetooth

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed, 15 Oct 2025 18:40:11 +0300 you wrote:
> Ordering of STREAMING and iso_connect_cb() is unspecified, as
> kernel/Glib do not preserve the order of these events.
> 
> STREAMING before iso_connect_cb() causes transport to fail Acquire()
> since bt_bap_stream_io::connecting == true.
> 
> Fix by marking IO as connected even though the connection didn't yet
> complete. The socket fd is valid, although not yet writable/readable.
> 
> [...]

Here is the summary with links:
  - [BlueZ,1/2] bap: tolerate inverted STREAMING and iso_connect_cb()
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c584335d4436
  - [BlueZ,2/2] transport: wait until BAP fd is writable/readable before resuming
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=bac077c96b67

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-20 13:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-15 15:40 [PATCH BlueZ 1/2] bap: tolerate inverted STREAMING and iso_connect_cb() Pauli Virtanen
2025-10-15 15:40 ` [PATCH BlueZ 2/2] transport: wait until BAP fd is writable/readable before resuming Pauli Virtanen
2025-10-15 17:35 ` [BlueZ,1/2] bap: tolerate inverted STREAMING and iso_connect_cb() bluez.test.bot
2025-10-20 13:20 ` [PATCH BlueZ 1/2] " patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox