Commit c096358e authored by Fam Zheng's avatar Fam Zheng Committed by Paolo Bonzini
Browse files

qemu-thread: Assert locks are initialized before using 



Not all platforms check whether a lock is initialized before used.  In
particular Linux seems to be more permissive than OSX.

Check initialization state explicitly in our code to catch such bugs
earlier.

Signed-off-by: default avatarFam Zheng <famz@redhat.com>
Message-Id: <20170704122325.25634-1-famz@redhat.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 025bdeab

include/qemu/thread-posix.h

+4 −0
Original line number Diff line number Diff line
@@ -12,10 +12,12 @@ typedef QemuMutex QemuRecMutex; 


struct QemuMutex {

    pthread_mutex_t lock;

    bool initialized;

};



struct QemuCond {

    pthread_cond_t cond;

    bool initialized;

};



struct QemuSemaphore {
@@ -26,6 +28,7 @@ struct QemuSemaphore { 
#else

    sem_t sem;

#endif

    bool initialized;

};



struct QemuEvent {
@@ -34,6 +37,7 @@ struct QemuEvent { 
    pthread_cond_t cond;

#endif

    unsigned value;

    bool initialized;

};



struct QemuThread {

include/qemu/thread-win32.h

+5 −0
Original line number Diff line number Diff line
@@ -5,11 +5,13 @@ 


struct QemuMutex {

    SRWLOCK lock;

    bool initialized;

};



typedef struct QemuRecMutex QemuRecMutex;

struct QemuRecMutex {

    CRITICAL_SECTION lock;

    bool initialized;

};



void qemu_rec_mutex_destroy(QemuRecMutex *mutex);
@@ -19,15 +21,18 @@ void qemu_rec_mutex_unlock(QemuRecMutex *mutex); 


struct QemuCond {

    CONDITION_VARIABLE var;

    bool initialized;

};



struct QemuSemaphore {

    HANDLE sema;

    bool initialized;

};



struct QemuEvent {

    int value;

    HANDLE event;

    bool initialized;

};



typedef struct QemuThreadData QemuThreadData;

util/qemu-thread-posix.c

+27 −0
Original line number Diff line number Diff line
@@ -43,12 +43,15 @@ void qemu_mutex_init(QemuMutex *mutex) 
    err = pthread_mutex_init(&mutex->lock, NULL);

    if (err)

        error_exit(err, __func__);

    mutex->initialized = true;

}



void qemu_mutex_destroy(QemuMutex *mutex)

{

    int err;



    assert(mutex->initialized);

    mutex->initialized = false;

    err = pthread_mutex_destroy(&mutex->lock);

    if (err)

        error_exit(err, __func__);
@@ -58,6 +61,7 @@ void qemu_mutex_lock(QemuMutex *mutex) 
{

    int err;



    assert(mutex->initialized);

    err = pthread_mutex_lock(&mutex->lock);

    if (err)

        error_exit(err, __func__);
@@ -69,6 +73,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) 
{

    int err;



    assert(mutex->initialized);

    err = pthread_mutex_trylock(&mutex->lock);

    if (err == 0) {

        trace_qemu_mutex_locked(mutex);
@@ -84,6 +89,7 @@ void qemu_mutex_unlock(QemuMutex *mutex) 
{

    int err;



    assert(mutex->initialized);

    trace_qemu_mutex_unlocked(mutex);

    err = pthread_mutex_unlock(&mutex->lock);

    if (err)
@@ -102,6 +108,7 @@ void qemu_rec_mutex_init(QemuRecMutex *mutex) 
    if (err) {

        error_exit(err, __func__);

    }

    mutex->initialized = true;

}



void qemu_cond_init(QemuCond *cond)
@@ -111,12 +118,15 @@ void qemu_cond_init(QemuCond *cond) 
    err = pthread_cond_init(&cond->cond, NULL);

    if (err)

        error_exit(err, __func__);

    cond->initialized = true;

}



void qemu_cond_destroy(QemuCond *cond)

{

    int err;



    assert(cond->initialized);

    cond->initialized = false;

    err = pthread_cond_destroy(&cond->cond);

    if (err)

        error_exit(err, __func__);
@@ -126,6 +136,7 @@ void qemu_cond_signal(QemuCond *cond) 
{

    int err;



    assert(cond->initialized);

    err = pthread_cond_signal(&cond->cond);

    if (err)

        error_exit(err, __func__);
@@ -135,6 +146,7 @@ void qemu_cond_broadcast(QemuCond *cond) 
{

    int err;



    assert(cond->initialized);

    err = pthread_cond_broadcast(&cond->cond);

    if (err)

        error_exit(err, __func__);
@@ -144,6 +156,7 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) 
{

    int err;



    assert(cond->initialized);

    trace_qemu_mutex_unlocked(mutex);

    err = pthread_cond_wait(&cond->cond, &mutex->lock);

    trace_qemu_mutex_locked(mutex);
@@ -174,12 +187,15 @@ void qemu_sem_init(QemuSemaphore *sem, int init) 
        error_exit(errno, __func__);

    }

#endif

    sem->initialized = true;

}



void qemu_sem_destroy(QemuSemaphore *sem)

{

    int rc;



    assert(sem->initialized);

    sem->initialized = false;

#if defined(__APPLE__) || defined(__NetBSD__)

    rc = pthread_cond_destroy(&sem->cond);

    if (rc < 0) {
@@ -201,6 +217,7 @@ void qemu_sem_post(QemuSemaphore *sem) 
{

    int rc;



    assert(sem->initialized);

#if defined(__APPLE__) || defined(__NetBSD__)

    pthread_mutex_lock(&sem->lock);

    if (sem->count == UINT_MAX) {
@@ -238,6 +255,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) 
    int rc;

    struct timespec ts;



    assert(sem->initialized);

#if defined(__APPLE__) || defined(__NetBSD__)

    rc = 0;

    compute_abs_deadline(&ts, ms);
@@ -285,6 +303,7 @@ void qemu_sem_wait(QemuSemaphore *sem) 
{

    int rc;



    assert(sem->initialized);

#if defined(__APPLE__) || defined(__NetBSD__)

    pthread_mutex_lock(&sem->lock);

    while (sem->count == 0) {
@@ -310,6 +329,7 @@ void qemu_sem_wait(QemuSemaphore *sem) 
#else

static inline void qemu_futex_wake(QemuEvent *ev, int n)

{

    assert(ev->initialized);

    pthread_mutex_lock(&ev->lock);

    if (n == 1) {

        pthread_cond_signal(&ev->cond);
@@ -321,6 +341,7 @@ static inline void qemu_futex_wake(QemuEvent *ev, int n) 


static inline void qemu_futex_wait(QemuEvent *ev, unsigned val)

{

    assert(ev->initialized);

    pthread_mutex_lock(&ev->lock);

    if (ev->value == val) {

        pthread_cond_wait(&ev->cond, &ev->lock);
@@ -355,10 +376,13 @@ void qemu_event_init(QemuEvent *ev, bool init) 
#endif



    ev->value = (init ? EV_SET : EV_FREE);

    ev->initialized = true;

}



void qemu_event_destroy(QemuEvent *ev)

{

    assert(ev->initialized);

    ev->initialized = false;

#ifndef __linux__

    pthread_mutex_destroy(&ev->lock);

    pthread_cond_destroy(&ev->cond);
@@ -370,6 +394,7 @@ void qemu_event_set(QemuEvent *ev) 
    /* qemu_event_set has release semantics, but because it *loads*

     * ev->value we need a full memory barrier here.

     */

    assert(ev->initialized);

    smp_mb();

    if (atomic_read(&ev->value) != EV_SET) {

        if (atomic_xchg(&ev->value, EV_SET) == EV_BUSY) {
@@ -383,6 +408,7 @@ void qemu_event_reset(QemuEvent *ev) 
{

    unsigned value;



    assert(ev->initialized);

    value = atomic_read(&ev->value);

    smp_mb_acquire();

    if (value == EV_SET) {
@@ -398,6 +424,7 @@ void qemu_event_wait(QemuEvent *ev) 
{

    unsigned value;



    assert(ev->initialized);

    value = atomic_read(&ev->value);

    smp_mb_acquire();

    if (value != EV_SET) {

util/qemu-thread-win32.c

+33 −1
Original line number Diff line number Diff line
@@ -46,15 +46,19 @@ static void error_exit(int err, const char *msg) 
void qemu_mutex_init(QemuMutex *mutex)

{

    InitializeSRWLock(&mutex->lock);

    mutex->initialized = true;

}



void qemu_mutex_destroy(QemuMutex *mutex)

{

    assert(mutex->initialized);

    mutex->initialized = false;

    InitializeSRWLock(&mutex->lock);

}



void qemu_mutex_lock(QemuMutex *mutex)

{

    assert(mutex->initialized);

    AcquireSRWLockExclusive(&mutex->lock);

    trace_qemu_mutex_locked(mutex);

}
@@ -63,6 +67,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) 
{

    int owned;



    assert(mutex->initialized);

    owned = TryAcquireSRWLockExclusive(&mutex->lock);

    if (owned) {

        trace_qemu_mutex_locked(mutex);
@@ -73,6 +78,7 @@ int qemu_mutex_trylock(QemuMutex *mutex) 


void qemu_mutex_unlock(QemuMutex *mutex)

{

    assert(mutex->initialized);

    trace_qemu_mutex_unlocked(mutex);

    ReleaseSRWLockExclusive(&mutex->lock);

}
@@ -80,25 +86,31 @@ void qemu_mutex_unlock(QemuMutex *mutex) 
void qemu_rec_mutex_init(QemuRecMutex *mutex)

{

    InitializeCriticalSection(&mutex->lock);

    mutex->initialized = true;

}



void qemu_rec_mutex_destroy(QemuRecMutex *mutex)

{

    assert(mutex->initialized);

    mutex->initialized = false;

    DeleteCriticalSection(&mutex->lock);

}



void qemu_rec_mutex_lock(QemuRecMutex *mutex)

{

    assert(mutex->initialized);

    EnterCriticalSection(&mutex->lock);

}



int qemu_rec_mutex_trylock(QemuRecMutex *mutex)

{

    assert(mutex->initialized);

    return !TryEnterCriticalSection(&mutex->lock);

}



void qemu_rec_mutex_unlock(QemuRecMutex *mutex)

{

    assert(mutex->initialized);

    LeaveCriticalSection(&mutex->lock);

}
@@ -106,25 +118,31 @@ void qemu_cond_init(QemuCond *cond) 
{

    memset(cond, 0, sizeof(*cond));

    InitializeConditionVariable(&cond->var);

    cond->initialized = true;

}



void qemu_cond_destroy(QemuCond *cond)

{

    assert(cond->initialized);

    cond->initialized = false;

    InitializeConditionVariable(&cond->var);

}



void qemu_cond_signal(QemuCond *cond)

{

    assert(cond->initialized);

    WakeConditionVariable(&cond->var);

}



void qemu_cond_broadcast(QemuCond *cond)

{

    assert(cond->initialized);

    WakeAllConditionVariable(&cond->var);

}



void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex)

{

    assert(cond->initialized);

    trace_qemu_mutex_unlocked(mutex);

    SleepConditionVariableSRW(&cond->var, &mutex->lock, INFINITE, 0);

    trace_qemu_mutex_locked(mutex);
@@ -134,21 +152,28 @@ void qemu_sem_init(QemuSemaphore *sem, int init) 
{

    /* Manual reset.  */

    sem->sema = CreateSemaphore(NULL, init, LONG_MAX, NULL);

    sem->initialized = true;

}



void qemu_sem_destroy(QemuSemaphore *sem)

{

    assert(sem->initialized);

    sem->initialized = false;

    CloseHandle(sem->sema);

}



void qemu_sem_post(QemuSemaphore *sem)

{

    assert(sem->initialized);

    ReleaseSemaphore(sem->sema, 1, NULL);

}



int qemu_sem_timedwait(QemuSemaphore *sem, int ms)

{

    int rc = WaitForSingleObject(sem->sema, ms);

    int rc;



    assert(sem->initialized);

    rc = WaitForSingleObject(sem->sema, ms);

    if (rc == WAIT_OBJECT_0) {

        return 0;

    }
@@ -160,6 +185,7 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms) 


void qemu_sem_wait(QemuSemaphore *sem)

{

    assert(sem->initialized);

    if (WaitForSingleObject(sem->sema, INFINITE) != WAIT_OBJECT_0) {

        error_exit(GetLastError(), __func__);

    }
@@ -193,15 +219,19 @@ void qemu_event_init(QemuEvent *ev, bool init) 
    /* Manual reset.  */

    ev->event = CreateEvent(NULL, TRUE, TRUE, NULL);

    ev->value = (init ? EV_SET : EV_FREE);

    ev->initialized = true;

}



void qemu_event_destroy(QemuEvent *ev)

{

    assert(ev->initialized);

    ev->initialized = false;

    CloseHandle(ev->event);

}



void qemu_event_set(QemuEvent *ev)

{

    assert(ev->initialized);

    /* qemu_event_set has release semantics, but because it *loads*

     * ev->value we need a full memory barrier here.

     */
@@ -218,6 +248,7 @@ void qemu_event_reset(QemuEvent *ev) 
{

    unsigned value;



    assert(ev->initialized);

    value = atomic_read(&ev->value);

    smp_mb_acquire();

    if (value == EV_SET) {
@@ -232,6 +263,7 @@ void qemu_event_wait(QemuEvent *ev) 
{

    unsigned value;



    assert(ev->initialized);

    value = atomic_read(&ev->value);

    smp_mb_acquire();

    if (value != EV_SET) {