[Qemu-devel] [PATCH 4/4] ccid: make threads joinable

[Paolo Bonzini <pbonzini@redhat.com>]

Destroying a mutex that another thread might have just unlocked
is racy.  It usually works, but you cannot do that in general and
can lead to deadlocks or segfaults.  Change ccid to use joinable
threads instead.

(Also, qemu_mutex_init/qemu_cond_init were missing).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ccid-card-emulated.c |   26 +++++++++++---------------
 1 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c
index 9fe9db5..2d2ebce 100644
--- a/hw/ccid-card-emulated.c
+++ b/hw/ccid-card-emulated.c
@@ -120,6 +120,7 @@ struct EmulatedState {
     uint8_t  atr_length;
     QSIMPLEQ_HEAD(event_list, EmulEvent) event_list;
     QemuMutex event_list_mutex;
+    QemuThread event_thread_id;
     VReader *reader;
     QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list;
     QemuMutex vreader_mutex; /* and guest_apdu_list mutex */
@@ -127,8 +128,7 @@ struct EmulatedState {
     QemuCond handle_apdu_cond;
     int      pipe[2];
     int      quit_apdu_thread;
-    QemuMutex apdu_thread_quit_mutex;
-    QemuCond apdu_thread_quit_cond;
+    QemuThread apdu_thread_id;
 };
 
 static void emulated_apdu_from_guest(CCIDCardState *base,
@@ -271,9 +271,6 @@ static void *handle_apdu_thread(void* arg)
         }
         qemu_mutex_unlock(&card->vreader_mutex);
     }
-    qemu_mutex_lock(&card->apdu_thread_quit_mutex);
-    qemu_cond_signal(&card->apdu_thread_quit_cond);
-    qemu_mutex_unlock(&card->apdu_thread_quit_mutex);
     return NULL;
 }
 
@@ -489,7 +486,6 @@ static uint32_t parse_enumeration(char *str,
 static int emulated_initfn(CCIDCardState *base)
 {
     EmulatedState *card = DO_UPCAST(EmulatedState, base, base);
-    QemuThread thread_id;
     VCardEmulError ret;
     EnumTable *ptable;
 
@@ -541,9 +537,10 @@ static int emulated_initfn(CCIDCardState *base)
         printf("%s: failed to initialize vcard\n", EMULATED_DEV_NAME);
         return -1;
     }
-    qemu_thread_create(&thread_id, event_thread, card, QEMU_THREAD_DETACHED);
-    qemu_thread_create(&thread_id, handle_apdu_thread, card,
-                       QEMU_THREAD_DETACHED);
+    qemu_thread_create(&card->event_thread_id, event_thread, card,
+                       QEMU_THREAD_JOINABLE);
+    qemu_thread_create(&card->apdu_thread_id, handle_apdu_thread, card,
+                       QEMU_THREAD_JOINABLE);
     return 0;
 }
 
@@ -553,15 +550,14 @@ static int emulated_exitfn(CCIDCardState *base)
     VEvent *vevent = vevent_new(VEVENT_LAST, NULL, NULL);
 
     vevent_queue_vevent(vevent); /* stop vevent thread */
-    qemu_mutex_lock(&card->apdu_thread_quit_mutex);
+    qemu_thread_join(&card->event_thread_id);
+
     card->quit_apdu_thread = 1; /* stop handle_apdu thread */
     qemu_cond_signal(&card->handle_apdu_cond);
-    qemu_cond_wait(&card->apdu_thread_quit_cond,
-                      &card->apdu_thread_quit_mutex);
-    /* handle_apdu thread stopped, can destroy all of it's mutexes */
+    qemu_thread_join(&card->apdu_thread_id);
+
+    /* threads exited, can destroy all condvars/mutexes */
     qemu_cond_destroy(&card->handle_apdu_cond);
-    qemu_cond_destroy(&card->apdu_thread_quit_cond);
-    qemu_mutex_destroy(&card->apdu_thread_quit_mutex);
     qemu_mutex_destroy(&card->handle_apdu_mutex);
     qemu_mutex_destroy(&card->vreader_mutex);
     qemu_mutex_destroy(&card->event_list_mutex);
-- 
1.7.7.1