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

[Paolo Bonzini <pbonzini@redhat.com>]

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