diff mbox series

[BlueZ,2/4] shared/bass: Handle G_IO_HUP on io channels

Message ID 20231016154900.3094-3-iulia.tanasescu@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add Modify Source initial implementation | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Iulia Tanasescu Oct. 16, 2023, 3:48 p.m. UTC 
This adds watches to handle closed sockets

---
 src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/bass.h |  2 ++
 2 files changed, 61 insertions(+)

Comments

Luiz Augusto von Dentz Oct. 16, 2023, 5:04 p.m. UTC | #1 
Hi Iulia,

On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>
> This adds watches to handle closed sockets
>
> ---
>  src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/bass.h |  2 ++
>  2 files changed, 61 insertions(+)
>
> diff --git a/src/shared/bass.c b/src/shared/bass.c
> index 0ee3187d1..78103d463 100644
> --- a/src/shared/bass.c
> +++ b/src/shared/bass.c
> @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                 g_io_channel_unref(bcast_src->listen_io);
>                 bcast_src->listen_io = NULL;
>
> +               if (bcast_src->listen_io_id > 0) {
> +                       g_source_remove(bcast_src->listen_io_id);
> +                       bcast_src->listen_io_id  = 0;
> +               }
> +
>                 /* Close pa sync io */
>                 if (bcast_src->pa_sync_io) {
>                         g_io_channel_shutdown(bcast_src->pa_sync_io,
> @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                         bcast_src->pa_sync_io = NULL;
>                 }
>
> +               if (bcast_src->pa_sync_io_id > 0) {
> +                       g_source_remove(bcast_src->pa_sync_io_id);
> +                       bcast_src->pa_sync_io_id  = 0;
> +               }
> +
>                 for (i = 0; i < bcast_src->num_subgroups; i++)
>                         bcast_src->subgroup_data[i].bis_sync =
>                                 BT_BASS_BIG_SYNC_FAILED_BITMASK;
> @@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
>         return false;
>  }
>
> +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src = data;
> +
> +       DBG(bcast_src->bass, "PA sync io has been disconnected");
> +
> +       bcast_src->pa_sync_io_id = 0;
> +       bcast_src->pa_sync_io = NULL;
> +
> +       return FALSE;
> +}
>
>  static void confirm_cb(GIOChannel *io, gpointer user_data)
>  {
> @@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
>         bcast_src->pa_sync_io = io;
>         g_io_channel_ref(bcast_src->pa_sync_io);
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  = 0;
> +       }
> +
> +       bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
> +                               G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> +                               bcast_src);
> +
>         len = sizeof(qos);
>         memset(&qos, 0, len);
>
> @@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *value, size_t len)
>         return true;
>  }
>
> +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src = data;
> +
> +       DBG(bcast_src->bass, "Listen io has been disconnected");
> +
> +       bcast_src->listen_io_id = 0;
> +       bcast_src->listen_io = NULL;
> +
> +       return FALSE;
> +}
> +
>  static void bass_handle_add_src_op(struct bt_bass *bass,
>                                         struct gatt_db_attribute *attrib,
>                                         uint8_t opcode,
> @@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
>                 bcast_src->listen_io = io;
>                 g_io_channel_ref(bcast_src->listen_io);
>
> +               bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> +                                       G_IO_HUP | G_IO_NVAL,
> +                                       (GIOFunc)listen_io_disconnect_cb,
> +                                       bcast_src);

IO handling shall probably be kept outside of shared so we could
emulate it with use of socketpairs and unit test it, see how it was
done for bap.

>                 if (num_bis > 0 && !bcast_src->bises)
>                         bcast_src->bises = queue_new();
>         } else {
> @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
>                 g_io_channel_unref(bcast_src->listen_io);
>         }
>
> +       if (bcast_src->listen_io_id > 0) {
> +               g_source_remove(bcast_src->listen_io_id);
> +               bcast_src->listen_io_id  = 0;
> +       }
> +
>         if (bcast_src->pa_sync_io) {
>                 g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
>                 g_io_channel_unref(bcast_src->pa_sync_io);
>         }
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  = 0;
> +       }
> +
>         queue_destroy(bcast_src->bises, bass_bis_unref);
>
>         free(bcast_src);
> diff --git a/src/shared/bass.h b/src/shared/bass.h
> index c4b5b76ba..bd3fe900b 100644
> --- a/src/shared/bass.h
> +++ b/src/shared/bass.h
> @@ -57,7 +57,9 @@ struct bt_bcast_src {
>         uint8_t num_subgroups;
>         struct bt_bass_subgroup_data *subgroup_data;
>         GIOChannel *listen_io;
> +       guint listen_io_id;
>         GIOChannel *pa_sync_io;
> +       guint pa_sync_io_id;
>         struct queue *bises;
>  };
>
> --
> 2.39.2
>
Iulia Tanasescu Oct. 25, 2023, 3 p.m. UTC | #2 
Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Monday, October 16, 2023 8:05 PM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>;
> Andrei Istodorescu <andrei.istodorescu@nxp.com>
> Subject: Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io
> channels
> 
> Hi Iulia,
> 
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com>
> wrote:
> >
> > This adds watches to handle closed sockets
> >
> > ---
> >  src/shared/bass.c | 59
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  src/shared/bass.h |  2 ++
> >  2 files changed, 61 insertions(+)
> >
> > diff --git a/src/shared/bass.c b/src/shared/bass.c index
> > 0ee3187d1..78103d463 100644
> > --- a/src/shared/bass.c
> > +++ b/src/shared/bass.c
> > @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> >                 g_io_channel_unref(bcast_src->listen_io);
> >                 bcast_src->listen_io = NULL;
> >
> > +               if (bcast_src->listen_io_id > 0) {
> > +                       g_source_remove(bcast_src->listen_io_id);
> > +                       bcast_src->listen_io_id  = 0;
> > +               }
> > +
> >                 /* Close pa sync io */
> >                 if (bcast_src->pa_sync_io) {
> >                         g_io_channel_shutdown(bcast_src->pa_sync_io,
> > @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> >                         bcast_src->pa_sync_io = NULL;
> >                 }
> >
> > +               if (bcast_src->pa_sync_io_id > 0) {
> > +                       g_source_remove(bcast_src->pa_sync_io_id);
> > +                       bcast_src->pa_sync_io_id  = 0;
> > +               }
> > +
> >                 for (i = 0; i < bcast_src->num_subgroups; i++)
> >                         bcast_src->subgroup_data[i].bis_sync =
> >                                 BT_BASS_BIG_SYNC_FAILED_BITMASK; @@
> > -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src
> *bcast_src)
> >         return false;
> >  }
> >
> > +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io,
> GIOCondition cond,
> > +                       gpointer data) {
> > +       struct bt_bcast_src *bcast_src = data;
> > +
> > +       DBG(bcast_src->bass, "PA sync io has been disconnected");
> > +
> > +       bcast_src->pa_sync_io_id = 0;
> > +       bcast_src->pa_sync_io = NULL;
> > +
> > +       return FALSE;
> > +}
> >
> >  static void confirm_cb(GIOChannel *io, gpointer user_data)  { @@
> > -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer
> user_data)
> >         bcast_src->pa_sync_io = io;
> >         g_io_channel_ref(bcast_src->pa_sync_io);
> >
> > +       if (bcast_src->pa_sync_io_id > 0) {
> > +               g_source_remove(bcast_src->pa_sync_io_id);
> > +               bcast_src->pa_sync_io_id  = 0;
> > +       }
> > +
> > +       bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR |
> G_IO_HUP |
> > +                               G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> > +                               bcast_src);
> > +
> >         len = sizeof(qos);
> >         memset(&qos, 0, len);
> >
> > @@ -844,6 +875,19 @@ static bool
> bass_validate_add_src_params(uint8_t *value, size_t len)
> >         return true;
> >  }
> >
> > +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition
> cond,
> > +                       gpointer data) {
> > +       struct bt_bcast_src *bcast_src = data;
> > +
> > +       DBG(bcast_src->bass, "Listen io has been disconnected");
> > +
> > +       bcast_src->listen_io_id = 0;
> > +       bcast_src->listen_io = NULL;
> > +
> > +       return FALSE;
> > +}
> > +
> >  static void bass_handle_add_src_op(struct bt_bass *bass,
> >                                         struct gatt_db_attribute *attrib,
> >                                         uint8_t opcode, @@ -1023,6
> > +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
> >                 bcast_src->listen_io = io;
> >                 g_io_channel_ref(bcast_src->listen_io);
> >
> > +               bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> > +                                       G_IO_HUP | G_IO_NVAL,
> > +                                       (GIOFunc)listen_io_disconnect_cb,
> > +                                       bcast_src);
> 
> IO handling shall probably be kept outside of shared so we could emulate it
> with use of socketpairs and unit test it, see how it was done for bap.
> 

I submitted a patch to move all IO handling from src/shared to
profiles/audio, similar to bap.

> >                 if (num_bis > 0 && !bcast_src->bises)
> >                         bcast_src->bises = queue_new();
> >         } else {
> > @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
> >                 g_io_channel_unref(bcast_src->listen_io);
> >         }
> >
> > +       if (bcast_src->listen_io_id > 0) {
> > +               g_source_remove(bcast_src->listen_io_id);
> > +               bcast_src->listen_io_id  = 0;
> > +       }
> > +
> >         if (bcast_src->pa_sync_io) {
> >                 g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
> >                 g_io_channel_unref(bcast_src->pa_sync_io);
> >         }
> >
> > +       if (bcast_src->pa_sync_io_id > 0) {
> > +               g_source_remove(bcast_src->pa_sync_io_id);
> > +               bcast_src->pa_sync_io_id  = 0;
> > +       }
> > +
> >         queue_destroy(bcast_src->bises, bass_bis_unref);
> >
> >         free(bcast_src);
> > diff --git a/src/shared/bass.h b/src/shared/bass.h index
> > c4b5b76ba..bd3fe900b 100644
> > --- a/src/shared/bass.h
> > +++ b/src/shared/bass.h
> > @@ -57,7 +57,9 @@ struct bt_bcast_src {
> >         uint8_t num_subgroups;
> >         struct bt_bass_subgroup_data *subgroup_data;
> >         GIOChannel *listen_io;
> > +       guint listen_io_id;
> >         GIOChannel *pa_sync_io;
> > +       guint pa_sync_io_id;
> >         struct queue *bises;
> >  };
> >
> > --
> > 2.39.2
> >
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Iulia
diff mbox series

Patch

diff --git a/src/shared/bass.c b/src/shared/bass.c
index 0ee3187d1..78103d463 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -655,6 +655,11 @@  static void connect_cb(GIOChannel *io, GError *gerr,
 		g_io_channel_unref(bcast_src->listen_io);
 		bcast_src->listen_io = NULL;
 
+		if (bcast_src->listen_io_id > 0) {
+			g_source_remove(bcast_src->listen_io_id);
+			bcast_src->listen_io_id  = 0;
+		}
+
 		/* Close pa sync io */
 		if (bcast_src->pa_sync_io) {
 			g_io_channel_shutdown(bcast_src->pa_sync_io,
@@ -663,6 +668,11 @@  static void connect_cb(GIOChannel *io, GError *gerr,
 			bcast_src->pa_sync_io = NULL;
 		}
 
+		if (bcast_src->pa_sync_io_id > 0) {
+			g_source_remove(bcast_src->pa_sync_io_id);
+			bcast_src->pa_sync_io_id  = 0;
+		}
+
 		for (i = 0; i < bcast_src->num_subgroups; i++)
 			bcast_src->subgroup_data[i].bis_sync =
 				BT_BASS_BIG_SYNC_FAILED_BITMASK;
@@ -703,6 +713,18 @@  static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
 	return false;
 }
 
+static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+			gpointer data)
+{
+	struct bt_bcast_src *bcast_src = data;
+
+	DBG(bcast_src->bass, "PA sync io has been disconnected");
+
+	bcast_src->pa_sync_io_id = 0;
+	bcast_src->pa_sync_io = NULL;
+
+	return FALSE;
+}
 
 static void confirm_cb(GIOChannel *io, gpointer user_data)
 {
@@ -726,6 +748,15 @@  static void confirm_cb(GIOChannel *io, gpointer user_data)
 	bcast_src->pa_sync_io = io;
 	g_io_channel_ref(bcast_src->pa_sync_io);
 
+	if (bcast_src->pa_sync_io_id > 0) {
+		g_source_remove(bcast_src->pa_sync_io_id);
+		bcast_src->pa_sync_io_id  = 0;
+	}
+
+	bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
+				G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
+				bcast_src);
+
 	len = sizeof(qos);
 	memset(&qos, 0, len);
 
@@ -844,6 +875,19 @@  static bool bass_validate_add_src_params(uint8_t *value, size_t len)
 	return true;
 }
 
+static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+			gpointer data)
+{
+	struct bt_bcast_src *bcast_src = data;
+
+	DBG(bcast_src->bass, "Listen io has been disconnected");
+
+	bcast_src->listen_io_id = 0;
+	bcast_src->listen_io = NULL;
+
+	return FALSE;
+}
+
 static void bass_handle_add_src_op(struct bt_bass *bass,
 					struct gatt_db_attribute *attrib,
 					uint8_t opcode,
@@ -1023,6 +1067,11 @@  static void bass_handle_add_src_op(struct bt_bass *bass,
 		bcast_src->listen_io = io;
 		g_io_channel_ref(bcast_src->listen_io);
 
+		bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
+					G_IO_HUP | G_IO_NVAL,
+					(GIOFunc)listen_io_disconnect_cb,
+					bcast_src);
+
 		if (num_bis > 0 && !bcast_src->bises)
 			bcast_src->bises = queue_new();
 	} else {
@@ -1318,11 +1367,21 @@  static void bass_bcast_src_free(void *data)
 		g_io_channel_unref(bcast_src->listen_io);
 	}
 
+	if (bcast_src->listen_io_id > 0) {
+		g_source_remove(bcast_src->listen_io_id);
+		bcast_src->listen_io_id  = 0;
+	}
+
 	if (bcast_src->pa_sync_io) {
 		g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
 		g_io_channel_unref(bcast_src->pa_sync_io);
 	}
 
+	if (bcast_src->pa_sync_io_id > 0) {
+		g_source_remove(bcast_src->pa_sync_io_id);
+		bcast_src->pa_sync_io_id  = 0;
+	}
+
 	queue_destroy(bcast_src->bises, bass_bis_unref);
 
 	free(bcast_src);
diff --git a/src/shared/bass.h b/src/shared/bass.h
index c4b5b76ba..bd3fe900b 100644
--- a/src/shared/bass.h
+++ b/src/shared/bass.h
@@ -57,7 +57,9 @@  struct bt_bcast_src {
 	uint8_t num_subgroups;
 	struct bt_bass_subgroup_data *subgroup_data;
 	GIOChannel *listen_io;
+	guint listen_io_id;
 	GIOChannel *pa_sync_io;
+	guint pa_sync_io_id;
 	struct queue *bises;
 };