Re: [Qemu-devel] [PATCH 1/2] iothread: stash thread ID away

[Paolo Bonzini]

Il 25/02/2014 16:42, Stefan Hajnoczi ha scritto:
> 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.

Yes.

> POSIX does say that:
>
> "It shall be safe to destroy an initialized mutex that is unlocked."

The question is what "unlocked" means... :)

> 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.

I think other races are possible.  Let's look at the simple lock in 
nptl/lowlevellock.h:

/* Mutex lock counter:
   bit 31 clear means unlocked;
   bit 31 set means locked.

   All code that looks at bit 31 first increases the 'number of
   interested threads' usage counter, which is in bits 0-30.

The comment is wrong, there is a fast path that does not do that; I'm 
not sure if this is why the problem can happen, I'm just pointing this 
out because it contradicts the code I'm posting now.

The file uses C code, but it's simpler to look at it in assembly. 
Unlocking is very simple:

                lock; btcl $31, futex
                jz 2f
                ... do futex wake ...
        2:

Locking has a fast path followed by preparing the slow path, re-checking 
the fastpath condition, and waiting if it fails still:

                lock; btsl $31, futex
                jnc 9f
                lock; incl futex
        1:
                lock; btsl $31, futex
                jnc 8f
                ... do futex wait ...
                jmp 1b
        8:
                lock; decl futex
        9:

It's possible, if futex is locked by CPU 0 and CPU 1 tries to grab it, 
that the following happens:

        CPU 0                           CPU 1
                                        lock; btsl $31, futex (fails)
                                        lock; incl futex
        lock; btcl %0 (not zero)
                                        lock; btsl $31, futex (succeeds)
                                        lock; decl futex

                                        destroy lock
                                        free(lock)
        futex wake

If you get an EFAULT from the futex wakeup, this could be a problem.

Paolo