Re: [Qemu-devel] [PATCH 1/2] qemu-thread: do not use PTHREAD_MUTEX_ERRORCHECK

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On 03/06/2015 02:22 AM, Paolo Bonzini wrote:
> PTHREAD_MUTEX_ERRORCHECK is completely broken with respect to fork.
> The way to safely do fork is to bring all threads to a quiescent
> state by acquiring locks (either in callers---as we do for the
> iothread mutex---or using pthread_atfork's prepare callbacks)
> and then release them in the child.

That, and POSIX itself says that pthread_atfork is a dangerous API, and
should not be used if at all possible, because it is broken by design.

> 
> The problem is that releasing error-checking locks in the child
> fails under glibc with EPERM, because the mutex stores a different
> owner tid than the duplicated thread in the child process.

Is that a bug in glibc?

>  We
> could make it work for locks acquired via pthread_atfork, by
> recreating the mutex in the child instead of unlocking it
> (we know that there are no other threads that could have taken
> the mutex; but when the lock is acquired in fork's caller
> that would not be possible.
> 
> The simplest solution is just to forgo error checking.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  util/qemu-thread-posix.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)

I'm not sure that weakening things is always the wisest idea, but you've
provided some good arguments for why we want it here, so:

Reviewed-by: Eric Blake <eblake@redhat.com>


> 
> diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
> index 50a29d8..ba67cec 100644
> --- a/util/qemu-thread-posix.c
> +++ b/util/qemu-thread-posix.c
> @@ -51,12 +51,8 @@ static void error_exit(int err, const char *msg)
>  void qemu_mutex_init(QemuMutex *mutex)
>  {
>      int err;
> -    pthread_mutexattr_t mutexattr;
>  
> -    pthread_mutexattr_init(&mutexattr);
> -    pthread_mutexattr_settype(&mutexattr, PTHREAD_MUTEX_ERRORCHECK);
> -    err = pthread_mutex_init(&mutex->lock, &mutexattr);
> -    pthread_mutexattr_destroy(&mutexattr);
> +    err = pthread_mutex_init(&mutex->lock, NULL);
>      if (err)
>          error_exit(err, __func__);
>  }
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]