[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] iothread: wait until the glib context is ac
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] iothread: wait until the glib context is acquired |
Date: |
Fri, 22 Feb 2019 11:29:06 +0800 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
On Thu, Feb 21, 2019 at 11:39:32AM +0100, Marc-André Lureau wrote:
> Hi
>
> On Thu, Feb 21, 2019 at 9:00 AM Peter Xu <address@hidden> wrote:
> >
> > On Wed, Feb 20, 2019 at 05:06:25PM +0100, Marc-André Lureau wrote:
> > > Another thread may acquire the glib context (temporarily) before
> > > g_main_context_push_thread_default().
> > >
> > > This can happen with the following qemu_chr_fe_set_handlers()
> > > modifications.
> > >
> > > Unfortunately, g_main_context_wait() is deprecated in glib
> > > 2.58 (apparently it was a broken interface). Use a polling loop.
> > >
> > > Signed-off-by: Marc-André Lureau <address@hidden>
> > > ---
> > > iothread.c | 7 +++++++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/iothread.c b/iothread.c
> > > index e615b7ae52..93cc3aa875 100644
> > > --- a/iothread.c
> > > +++ b/iothread.c
> > > @@ -70,6 +70,11 @@ static void *iothread_run(void *opaque)
> > > if (iothread->running && atomic_read(&iothread->worker_context))
> > > {
> > > GMainLoop *loop;
> > >
> > > + /* we may race with another thread acquiring the context */
> > > + while (!g_main_context_acquire(iothread->worker_context)) {
> > > + g_usleep(10000);
> > > + }
> >
> > Could you help explain why need this explicitly? Since AFAIU
> > g_main_loop_run() below will do context acquire too so IIUC you're
> > taking it twice (while g_main_context_acquire should allow it to
> > happen, though)?
> >
>
> We call g_main_context_push_thread_default() before run(). It will
> fail if the context is not acquirable.
Thanks for explaining. It wasn't obvious to me.
I've posted another series to refactor iothread a bit and it should be
able to drop this patch if based on that series (that series should
even remove code instead of adding new). Please feel free to have a
look, or give it a shot:
[PATCH 0/4] iothread: create gcontext unconditionally
Regards,
--
Peter Xu
[Qemu-devel] [PATCH 3/4] monitor: set the chardev context from the main context/thread, Marc-André Lureau, 2019/02/20
[Qemu-devel] [PATCH 4/4] char-socket: restart the reconnect timer to switch context, Marc-André Lureau, 2019/02/20
Re: [Qemu-devel] [PATCH 0/4] RFC: make chardev context switching safer, Peter Xu, 2019/02/21