Message ID | 20180129161708.13616-1-quintela@redhat.com |
---|---|
State | New |
Headers | show |
Series | char-pty: fix glib assert | expand |
On 29/01/2018 17:17, Juan Quintela wrote: > I am having this assert over and over: > > (process:30804): GLib-CRITICAL **: g_source_unref: assertion 'source != NULL' failed > > gdb points to the line changed on this patch, and my reading of > commit: > > 2c716ba1506769c9be2caa02f0f6d6e7c00f4304 > > is that it should be timer_src what is unrefered there. > > But I don't claim to fully understand this code, so .... > > Any comment? > > Thanks, Juan. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > CC: Peter Xu <peterx@redhat.com> > CC: Paolo Bonzini <pbonzini@redhat.com> > > > --- > chardev/char-pty.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c > index 89315e6807..c26e02bce8 100644 > --- a/chardev/char-pty.c > +++ b/chardev/char-pty.c > @@ -57,8 +57,8 @@ static gboolean pty_chr_timer(gpointer opaque) > PtyChardev *s = PTY_CHARDEV(opaque); > > qemu_mutex_lock(&chr->chr_write_lock); > + g_source_unref(s->timer_src); > s->timer_src = NULL; > - g_source_unref(s->open_source); > s->open_source = NULL; > if (!s->connected) { > /* Next poll ... */ > If I read correctly the doc, we also need a g_source_destroy(s->timer_src). Perhaps we can directly call pty_chr_timer_cancel() here? Thanks, Laurent
On Mon, Jan 29, 2018 at 05:40:11PM +0100, Laurent Vivier wrote: > On 29/01/2018 17:17, Juan Quintela wrote: > > I am having this assert over and over: > > > > (process:30804): GLib-CRITICAL **: g_source_unref: assertion 'source != NULL' failed > > > > gdb points to the line changed on this patch, and my reading of > > commit: > > > > 2c716ba1506769c9be2caa02f0f6d6e7c00f4304 > > > > is that it should be timer_src what is unrefered there. > > > > But I don't claim to fully understand this code, so .... > > > > Any comment? > > > > Thanks, Juan. > > > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > CC: Peter Xu <peterx@redhat.com> > > CC: Paolo Bonzini <pbonzini@redhat.com> > > > > > > --- > > chardev/char-pty.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c > > index 89315e6807..c26e02bce8 100644 > > --- a/chardev/char-pty.c > > +++ b/chardev/char-pty.c > > @@ -57,8 +57,8 @@ static gboolean pty_chr_timer(gpointer opaque) > > PtyChardev *s = PTY_CHARDEV(opaque); > > > > qemu_mutex_lock(&chr->chr_write_lock); > > + g_source_unref(s->timer_src); > > s->timer_src = NULL; > > - g_source_unref(s->open_source); > > s->open_source = NULL; > > if (!s->connected) { > > /* Next poll ... */ > > > > If I read correctly the doc, we also need a > g_source_destroy(s->timer_src). Perhaps we can directly call > pty_chr_timer_cancel() here? Yes. Another fix is already queued by Paolo here: http://patchwork.ozlabs.org/patch/862763/ Sorry for that!
diff --git a/chardev/char-pty.c b/chardev/char-pty.c index 89315e6807..c26e02bce8 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -57,8 +57,8 @@ static gboolean pty_chr_timer(gpointer opaque) PtyChardev *s = PTY_CHARDEV(opaque); qemu_mutex_lock(&chr->chr_write_lock); + g_source_unref(s->timer_src); s->timer_src = NULL; - g_source_unref(s->open_source); s->open_source = NULL; if (!s->connected) { /* Next poll ... */
I am having this assert over and over: (process:30804): GLib-CRITICAL **: g_source_unref: assertion 'source != NULL' failed gdb points to the line changed on this patch, and my reading of commit: 2c716ba1506769c9be2caa02f0f6d6e7c00f4304 is that it should be timer_src what is unrefered there. But I don't claim to fully understand this code, so .... Any comment? Thanks, Juan. Signed-off-by: Juan Quintela <quintela@redhat.com> CC: Peter Xu <peterx@redhat.com> CC: Paolo Bonzini <pbonzini@redhat.com> --- chardev/char-pty.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)