GLib child source were added with version 2.28. We can use them now
that we bumped our requirement to 2.40.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
chardev/char-io.c | 48 +++++------------------------------------------
1 file changed, 5 insertions(+), 43 deletions(-)
diff --git a/chardev/char-io.c b/chardev/char-io.c
index f81052481a..8ced184160 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -33,7 +33,6 @@ typedef struct IOWatchPoll {
IOCanReadHandler *fd_can_read;
GSourceFunc fd_read;
void *opaque;
- GMainContext *context;
} IOWatchPoll;
static IOWatchPoll *io_watch_poll_from_source(GSource *source)
@@ -55,47 +54,24 @@ static gboolean io_watch_poll_prepare(GSource *source,
iwp->src = qio_channel_create_watch(
iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
- g_source_attach(iwp->src, iwp->context);
- } else {
- g_source_destroy(iwp->src);
+ g_source_add_child_source(source, iwp->src);
g_source_unref(iwp->src);
+ } else {
+ g_source_remove_child_source(source, iwp->src);
iwp->src = NULL;
}
return FALSE;
}
-static gboolean io_watch_poll_check(GSource *source)
-{
- return FALSE;
-}
-
static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
gpointer user_data)
{
- abort();
-}
-
-static void io_watch_poll_finalize(GSource *source)
-{
- /* Due to a glib bug, removing the last reference to a source
- * inside a finalize callback causes recursive locking (and a
- * deadlock). This is not a problem inside other callbacks,
- * including dispatch callbacks, so we call io_remove_watch_poll
- * to remove this source. At this point, iwp->src must
- * be NULL, or we would leak it.
- *
- * This would be solved much more elegantly by child sources,
- * but we support older glib versions that do not have them.
- */
- IOWatchPoll *iwp = io_watch_poll_from_source(source);
- assert(iwp->src == NULL);
+ return G_SOURCE_CONTINUE;
}
static GSourceFuncs io_watch_poll_funcs = {
.prepare = io_watch_poll_prepare,
- .check = io_watch_poll_check,
.dispatch = io_watch_poll_dispatch,
- .finalize = io_watch_poll_finalize,
};
GSource *io_add_watch_poll(Chardev *chr,
@@ -115,7 +91,6 @@ GSource *io_add_watch_poll(Chardev *chr,
iwp->ioc = ioc;
iwp->fd_read = (GSourceFunc) fd_read;
iwp->src = NULL;
- iwp->context = context;
name = g_strdup_printf("chardev-iowatch-%s", chr->label);
g_source_set_name((GSource *)iwp, name);
@@ -126,23 +101,10 @@ GSource *io_add_watch_poll(Chardev *chr,
return (GSource *)iwp;
}
-static void io_remove_watch_poll(GSource *source)
-{
- IOWatchPoll *iwp;
-
- iwp = io_watch_poll_from_source(source);
- if (iwp->src) {
- g_source_destroy(iwp->src);
- g_source_unref(iwp->src);
- iwp->src = NULL;
- }
- g_source_destroy(&iwp->parent);
-}
-
void remove_fd_in_watch(Chardev *chr)
{
if (chr->gsource) {
- io_remove_watch_poll(chr->gsource);
+ g_source_destroy(chr->gsource);
chr->gsource = NULL;
}
}
--
2.18.0.547.g1d89318c48
On Tue, Aug 28, 2018 at 12:23:16AM +0200, Marc-André Lureau wrote: > GLib child source were added with version 2.28. We can use them now > that we bumped our requirement to 2.40. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > chardev/char-io.c | 48 +++++------------------------------------------ > 1 file changed, 5 insertions(+), 43 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Hi guys,
We have some random delays with gdbstub since this commit under Windows.
Basically while debugging the next command can sometimes take more than 5
seconds while it usually tooks 30ms before. It is not target dependent I see
that happening with sparc, ppc, riscv, etc.. but only on Windows host.
Does that ring a bell?
Cheers,
Fred
Le 8/28/18 à 12:23 AM, Marc-André Lureau a écrit :
> GLib child source were added with version 2.28. We can use them now
> that we bumped our requirement to 2.40.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char-io.c | 48 +++++------------------------------------------
> 1 file changed, 5 insertions(+), 43 deletions(-)
>
> diff --git a/chardev/char-io.c b/chardev/char-io.c
> index f81052481a..8ced184160 100644
> --- a/chardev/char-io.c
> +++ b/chardev/char-io.c
> @@ -33,7 +33,6 @@ typedef struct IOWatchPoll {
> IOCanReadHandler *fd_can_read;
> GSourceFunc fd_read;
> void *opaque;
> - GMainContext *context;
> } IOWatchPoll;
>
> static IOWatchPoll *io_watch_poll_from_source(GSource *source)
> @@ -55,47 +54,24 @@ static gboolean io_watch_poll_prepare(GSource *source,
> iwp->src = qio_channel_create_watch(
> iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
> g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
> - g_source_attach(iwp->src, iwp->context);
> - } else {
> - g_source_destroy(iwp->src);
> + g_source_add_child_source(source, iwp->src);
> g_source_unref(iwp->src);
> + } else {
> + g_source_remove_child_source(source, iwp->src);
> iwp->src = NULL;
> }
> return FALSE;
> }
>
> -static gboolean io_watch_poll_check(GSource *source)
> -{
> - return FALSE;
> -}
> -
> static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
> gpointer user_data)
> {
> - abort();
> -}
> -
> -static void io_watch_poll_finalize(GSource *source)
> -{
> - /* Due to a glib bug, removing the last reference to a source
> - * inside a finalize callback causes recursive locking (and a
> - * deadlock). This is not a problem inside other callbacks,
> - * including dispatch callbacks, so we call io_remove_watch_poll
> - * to remove this source. At this point, iwp->src must
> - * be NULL, or we would leak it.
> - *
> - * This would be solved much more elegantly by child sources,
> - * but we support older glib versions that do not have them.
> - */
> - IOWatchPoll *iwp = io_watch_poll_from_source(source);
> - assert(iwp->src == NULL);
> + return G_SOURCE_CONTINUE;
> }
>
> static GSourceFuncs io_watch_poll_funcs = {
> .prepare = io_watch_poll_prepare,
> - .check = io_watch_poll_check,
> .dispatch = io_watch_poll_dispatch,
> - .finalize = io_watch_poll_finalize,
> };
>
> GSource *io_add_watch_poll(Chardev *chr,
> @@ -115,7 +91,6 @@ GSource *io_add_watch_poll(Chardev *chr,
> iwp->ioc = ioc;
> iwp->fd_read = (GSourceFunc) fd_read;
> iwp->src = NULL;
> - iwp->context = context;
>
> name = g_strdup_printf("chardev-iowatch-%s", chr->label);
> g_source_set_name((GSource *)iwp, name);
> @@ -126,23 +101,10 @@ GSource *io_add_watch_poll(Chardev *chr,
> return (GSource *)iwp;
> }
>
> -static void io_remove_watch_poll(GSource *source)
> -{
> - IOWatchPoll *iwp;
> -
> - iwp = io_watch_poll_from_source(source);
> - if (iwp->src) {
> - g_source_destroy(iwp->src);
> - g_source_unref(iwp->src);
> - iwp->src = NULL;
> - }
> - g_source_destroy(&iwp->parent);
> -}
> -
> void remove_fd_in_watch(Chardev *chr)
> {
> if (chr->gsource) {
> - io_remove_watch_poll(chr->gsource);
> + g_source_destroy(chr->gsource);
> chr->gsource = NULL;
> }
> }
>
Hi
On Thu, Apr 4, 2019 at 5:18 PM KONRAD Frederic
<frederic.konrad@adacore.com> wrote:
>
> Hi guys,
>
> We have some random delays with gdbstub since this commit under Windows.
> Basically while debugging the next command can sometimes take more than 5
> seconds while it usually tooks 30ms before. It is not target dependent I see
> that happening with sparc, ppc, riscv, etc.. but only on Windows host.
>
> Does that ring a bell?
No, is this with a socket chardev or other kind?
Could you check the chardev behaviour with the HMP monitor for ex:
qemu -chardev socket,id=foo,server,host=localhost,port=9999 -monitor chardev:foo
Is this also taking a few seconds to reply?
Have you checked running tests/test-char on windows? Is this also
taking long (>2sec)?
>
> Cheers,
> Fred
>
> Le 8/28/18 à 12:23 AM, Marc-André Lureau a écrit :
> > GLib child source were added with version 2.28. We can use them now
> > that we bumped our requirement to 2.40.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > chardev/char-io.c | 48 +++++------------------------------------------
> > 1 file changed, 5 insertions(+), 43 deletions(-)
> >
> > diff --git a/chardev/char-io.c b/chardev/char-io.c
> > index f81052481a..8ced184160 100644
> > --- a/chardev/char-io.c
> > +++ b/chardev/char-io.c
> > @@ -33,7 +33,6 @@ typedef struct IOWatchPoll {
> > IOCanReadHandler *fd_can_read;
> > GSourceFunc fd_read;
> > void *opaque;
> > - GMainContext *context;
> > } IOWatchPoll;
> >
> > static IOWatchPoll *io_watch_poll_from_source(GSource *source)
> > @@ -55,47 +54,24 @@ static gboolean io_watch_poll_prepare(GSource *source,
> > iwp->src = qio_channel_create_watch(
> > iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
> > g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
> > - g_source_attach(iwp->src, iwp->context);
> > - } else {
> > - g_source_destroy(iwp->src);
> > + g_source_add_child_source(source, iwp->src);
> > g_source_unref(iwp->src);
> > + } else {
> > + g_source_remove_child_source(source, iwp->src);
> > iwp->src = NULL;
> > }
> > return FALSE;
> > }
> >
> > -static gboolean io_watch_poll_check(GSource *source)
> > -{
> > - return FALSE;
> > -}
> > -
> > static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
> > gpointer user_data)
> > {
> > - abort();
> > -}
> > -
> > -static void io_watch_poll_finalize(GSource *source)
> > -{
> > - /* Due to a glib bug, removing the last reference to a source
> > - * inside a finalize callback causes recursive locking (and a
> > - * deadlock). This is not a problem inside other callbacks,
> > - * including dispatch callbacks, so we call io_remove_watch_poll
> > - * to remove this source. At this point, iwp->src must
> > - * be NULL, or we would leak it.
> > - *
> > - * This would be solved much more elegantly by child sources,
> > - * but we support older glib versions that do not have them.
> > - */
> > - IOWatchPoll *iwp = io_watch_poll_from_source(source);
> > - assert(iwp->src == NULL);
> > + return G_SOURCE_CONTINUE;
> > }
> >
> > static GSourceFuncs io_watch_poll_funcs = {
> > .prepare = io_watch_poll_prepare,
> > - .check = io_watch_poll_check,
> > .dispatch = io_watch_poll_dispatch,
> > - .finalize = io_watch_poll_finalize,
> > };
> >
> > GSource *io_add_watch_poll(Chardev *chr,
> > @@ -115,7 +91,6 @@ GSource *io_add_watch_poll(Chardev *chr,
> > iwp->ioc = ioc;
> > iwp->fd_read = (GSourceFunc) fd_read;
> > iwp->src = NULL;
> > - iwp->context = context;
> >
> > name = g_strdup_printf("chardev-iowatch-%s", chr->label);
> > g_source_set_name((GSource *)iwp, name);
> > @@ -126,23 +101,10 @@ GSource *io_add_watch_poll(Chardev *chr,
> > return (GSource *)iwp;
> > }
> >
> > -static void io_remove_watch_poll(GSource *source)
> > -{
> > - IOWatchPoll *iwp;
> > -
> > - iwp = io_watch_poll_from_source(source);
> > - if (iwp->src) {
> > - g_source_destroy(iwp->src);
> > - g_source_unref(iwp->src);
> > - iwp->src = NULL;
> > - }
> > - g_source_destroy(&iwp->parent);
> > -}
> > -
> > void remove_fd_in_watch(Chardev *chr)
> > {
> > if (chr->gsource) {
> > - io_remove_watch_poll(chr->gsource);
> > + g_source_destroy(chr->gsource);
> > chr->gsource = NULL;
> > }
> > }
> >
Le 4/4/19 à 5:44 PM, Marc-André Lureau a écrit :
> Hi
>
> On Thu, Apr 4, 2019 at 5:18 PM KONRAD Frederic
> <frederic.konrad@adacore.com> wrote:
>>
>> Hi guys,
>>
>> We have some random delays with gdbstub since this commit under Windows.
>> Basically while debugging the next command can sometimes take more than 5
>> seconds while it usually tooks 30ms before. It is not target dependent I see
>> that happening with sparc, ppc, riscv, etc.. but only on Windows host.
>>
>> Does that ring a bell?
>
> No, is this with a socket chardev or other kind?
It's connecting through the TCP socket. '-s -S' option.
>
> Could you check the chardev behaviour with the HMP monitor for ex:
> qemu -chardev socket,id=foo,server,host=localhost,port=9999 -monitor chardev:foo
>
> Is this also taking a few seconds to reply?
>
> Have you checked running tests/test-char on windows? Is this also
> taking long (>2sec)?
I'll try to test that.
Thanks!
Fred
>
>>
>> Cheers,
>> Fred
>>
>> Le 8/28/18 à 12:23 AM, Marc-André Lureau a écrit :
>>> GLib child source were added with version 2.28. We can use them now
>>> that we bumped our requirement to 2.40.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>> chardev/char-io.c | 48 +++++------------------------------------------
>>> 1 file changed, 5 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/chardev/char-io.c b/chardev/char-io.c
>>> index f81052481a..8ced184160 100644
>>> --- a/chardev/char-io.c
>>> +++ b/chardev/char-io.c
>>> @@ -33,7 +33,6 @@ typedef struct IOWatchPoll {
>>> IOCanReadHandler *fd_can_read;
>>> GSourceFunc fd_read;
>>> void *opaque;
>>> - GMainContext *context;
>>> } IOWatchPoll;
>>>
>>> static IOWatchPoll *io_watch_poll_from_source(GSource *source)
>>> @@ -55,47 +54,24 @@ static gboolean io_watch_poll_prepare(GSource *source,
>>> iwp->src = qio_channel_create_watch(
>>> iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
>>> g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
>>> - g_source_attach(iwp->src, iwp->context);
>>> - } else {
>>> - g_source_destroy(iwp->src);
>>> + g_source_add_child_source(source, iwp->src);
>>> g_source_unref(iwp->src);
>>> + } else {
>>> + g_source_remove_child_source(source, iwp->src);
>>> iwp->src = NULL;
>>> }
>>> return FALSE;
>>> }
>>>
>>> -static gboolean io_watch_poll_check(GSource *source)
>>> -{
>>> - return FALSE;
>>> -}
>>> -
>>> static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
>>> gpointer user_data)
>>> {
>>> - abort();
>>> -}
>>> -
>>> -static void io_watch_poll_finalize(GSource *source)
>>> -{
>>> - /* Due to a glib bug, removing the last reference to a source
>>> - * inside a finalize callback causes recursive locking (and a
>>> - * deadlock). This is not a problem inside other callbacks,
>>> - * including dispatch callbacks, so we call io_remove_watch_poll
>>> - * to remove this source. At this point, iwp->src must
>>> - * be NULL, or we would leak it.
>>> - *
>>> - * This would be solved much more elegantly by child sources,
>>> - * but we support older glib versions that do not have them.
>>> - */
>>> - IOWatchPoll *iwp = io_watch_poll_from_source(source);
>>> - assert(iwp->src == NULL);
>>> + return G_SOURCE_CONTINUE;
>>> }
>>>
>>> static GSourceFuncs io_watch_poll_funcs = {
>>> .prepare = io_watch_poll_prepare,
>>> - .check = io_watch_poll_check,
>>> .dispatch = io_watch_poll_dispatch,
>>> - .finalize = io_watch_poll_finalize,
>>> };
>>>
>>> GSource *io_add_watch_poll(Chardev *chr,
>>> @@ -115,7 +91,6 @@ GSource *io_add_watch_poll(Chardev *chr,
>>> iwp->ioc = ioc;
>>> iwp->fd_read = (GSourceFunc) fd_read;
>>> iwp->src = NULL;
>>> - iwp->context = context;
>>>
>>> name = g_strdup_printf("chardev-iowatch-%s", chr->label);
>>> g_source_set_name((GSource *)iwp, name);
>>> @@ -126,23 +101,10 @@ GSource *io_add_watch_poll(Chardev *chr,
>>> return (GSource *)iwp;
>>> }
>>>
>>> -static void io_remove_watch_poll(GSource *source)
>>> -{
>>> - IOWatchPoll *iwp;
>>> -
>>> - iwp = io_watch_poll_from_source(source);
>>> - if (iwp->src) {
>>> - g_source_destroy(iwp->src);
>>> - g_source_unref(iwp->src);
>>> - iwp->src = NULL;
>>> - }
>>> - g_source_destroy(&iwp->parent);
>>> -}
>>> -
>>> void remove_fd_in_watch(Chardev *chr)
>>> {
>>> if (chr->gsource) {
>>> - io_remove_watch_poll(chr->gsource);
>>> + g_source_destroy(chr->gsource);
>>> chr->gsource = NULL;
>>> }
>>> }
>>>
Hi Marc, I did more experiments.. It seems to be an issue in glib-2.44.1 which is quite old. I updated and it seems I don't see the issue anymore. Thanks for your input on this! Cheers, Fred Le 4/4/19 à 5:49 PM, KONRAD Frederic a écrit : > > > Le 4/4/19 à 5:44 PM, Marc-André Lureau a écrit : >> Hi >> >> On Thu, Apr 4, 2019 at 5:18 PM KONRAD Frederic >> <frederic.konrad@adacore.com> wrote: >>> >>> Hi guys, >>> >>> We have some random delays with gdbstub since this commit under Windows. >>> Basically while debugging the next command can sometimes take more than 5 >>> seconds while it usually tooks 30ms before. It is not target dependent I see >>> that happening with sparc, ppc, riscv, etc.. but only on Windows host. >>> >>> Does that ring a bell? >> >> No, is this with a socket chardev or other kind? > > It's connecting through the TCP socket. '-s -S' option. > >> >> Could you check the chardev behaviour with the HMP monitor for ex: >> qemu -chardev socket,id=foo,server,host=localhost,port=9999 -monitor chardev:foo >> >> Is this also taking a few seconds to reply? >> >> Have you checked running tests/test-char on windows? Is this also >> taking long (>2sec)? > > I'll try to test that. > > Thanks! > Fred >
© 2016 - 2025 Red Hat, Inc.