On Fri, Sep 22, 2017 at 09:06:26PM +0800, Fam Zheng wrote:
> On Fri, 09/22 16:56, Peter Xu wrote:
> > So that internal iothread users can explicitly stop one iothread without
> > destroying it.
> >
> > Since at it, fix iothread_stop() to allow re-entrance. Before this
>
> I don't think there is any re-entrace here. Maybe you mean
>
> s/re-entrance/calling multiple times/
>
> ?
Yes, you are right.
>
> > patch we may call iothread_stop() twice on single iothread, while that
> > may not be correct since qemu_thread_join() is not allowed to run twice.
> > From manual of pthread_join():
>
> Is one call from iothread_stop_all() and one from object finalize?
Yes.
>
> >
> > Joining with a thread that has previously been joined results in
> > undefined behavior.
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > include/sysemu/iothread.h | 1 +
> > iothread.c | 24 ++++++++++++++++--------
> > 2 files changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h
> > index b07663f..110329b 100644
> > --- a/include/sysemu/iothread.h
> > +++ b/include/sysemu/iothread.h
> > @@ -52,6 +52,7 @@ GMainContext *iothread_get_g_main_context(IOThread
> > *iothread);
> > * "query-iothreads".
> > */
> > IOThread *iothread_create(const char *id, Error **errp);
> > +void iothread_stop(IOThread *iothread);
> > void iothread_destroy(IOThread *iothread);
> >
> > #endif /* IOTHREAD_H */
> > diff --git a/iothread.c b/iothread.c
> > index 74e400c..894756b 100644
> > --- a/iothread.c
> > +++ b/iothread.c
> > @@ -80,13 +80,10 @@ static void *iothread_run(void *opaque)
> > return NULL;
> > }
> >
> > -static int iothread_stop(Object *object, void *opaque)
> > +void iothread_stop(IOThread *iothread)
> > {
> > - IOThread *iothread;
> > -
> > - iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
> > - if (!iothread || !iothread->ctx) {
> > - return 0;
> > + if (iothread->stopping) {
> > + return;
> > }
> > iothread->stopping = true;
> > aio_notify(iothread->ctx);
> > @@ -94,6 +91,17 @@ static int iothread_stop(Object *object, void *opaque)
> > g_main_loop_quit(iothread->main_loop);
> > }
> > qemu_thread_join(&iothread->thread);
> > +}
> > +
> > +static int iothread_stop_iter(Object *object, void *opaque)
> > +{
> > + IOThread *iothread;
> > +
> > + iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD);
> > + if (!iothread || !iothread->ctx) {
> > + return 0;
> > + }
>
> I think the check of iothread->ctx can be moved to iothread_stop() too.
Yes, will do.
--
Peter Xu
