qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
	"Shergill, Gurinder" <gurinder.shergill@hp.com>,
	"Vinod, Chegu" <chegu_vinod@hp.com>,
	qemu-devel@nongnu.org, Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] iothread: stash thread ID away
Date: Tue, 25 Feb 2014 16:42:45 +0100	[thread overview]
Message-ID: <20140225154245.GC2374@stefanha-thinkpad.redhat.com> (raw)
In-Reply-To: <530B77CD.9020502@redhat.com>

On Mon, Feb 24, 2014 at 05:48:13PM +0100, Paolo Bonzini wrote:
> Il 24/02/2014 16:53, Stefan Hajnoczi ha scritto:
> >>>> >+    qemu_cond_destroy(&init_info.init_done_cond);
> >>>> >+    qemu_mutex_destroy(&init_info.init_done_lock);
> >>>
> >>> Destroying the mutex here is racy.  You need to keep it until the
> >>> iothread is destroyed.
> >I don't think so:
> >
> >qemu_cond_signal() is called with the mutex held.  Therefore, our
> >qemu_cond_wait() followed by qemu_mutex_unlock() will only complete once
> >the thread has released the mutex.
> >
> >The thread will never touch the mutex again so it is safe to destroy it.
> >There is no race condition.
> 
> Could qemu_mutex_destroy run while the other thread has already
> released the main thread, but before it returns?  As far as I know,
> the only time when it is safe to destroy the "last" synchronization
> object (in this case the mutex is the last, the condvar is not) is
> after pthread_join.

I guess you're saying that while unlocking the mutex is atomic, that
doesn't guarantee pthread won't access the mutex internal state some
more after it has unlocked it.  Therefore it's not safe for another
thread to destroy the mutex even after it has acquired it.

POSIX does say that:

"It shall be safe to destroy an initialized mutex that is unlocked."

But maybe I am reading too much into that?

After poking around glibc a little I think you are right.  I can't say
for sure but it seems even after a futex call glibc might still mess
with internal state.  But if anyone knows for certain, please speak up.

Stefan

  reply	other threads:[~2014-02-25 15:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-21 14:51 [Qemu-devel] [PATCH 0/2] dataplane: add query-iothreads QMP command Stefan Hajnoczi
2014-02-21 14:51 ` [Qemu-devel] [PATCH 1/2] iothread: stash thread ID away Stefan Hajnoczi
2014-02-21 15:18   ` Paolo Bonzini
2014-02-24 15:53     ` Stefan Hajnoczi
2014-02-24 16:48       ` Paolo Bonzini
2014-02-25 15:42         ` Stefan Hajnoczi [this message]
2014-02-25 16:10           ` Paolo Bonzini
2014-02-25 16:17         ` Stefan Hajnoczi
2014-02-25 16:27           ` Paolo Bonzini
2014-02-21 14:51 ` [Qemu-devel] [PATCH 2/2] qmp: add query-iothreads command Stefan Hajnoczi
2014-02-21 15:27   ` Eric Blake
2014-02-24 15:54     ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140225154245.GC2374@stefanha-thinkpad.redhat.com \
    --to=stefanha@redhat.com \
    --cc=chegu_vinod@hp.com \
    --cc=gurinder.shergill@hp.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).