From patchwork Mon Dec 12 16:21:33 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 130774 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EFE6E1007D1 for ; Tue, 13 Dec 2011 03:22:44 +1100 (EST) Received: from localhost ([::1]:58942 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ra8eI-0001Fu-7V for incoming@patchwork.ozlabs.org; Mon, 12 Dec 2011 11:22:42 -0500 Received: from eggs.gnu.org ([140.186.70.92]:53405) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ra8dc-0007rX-2T for qemu-devel@nongnu.org; Mon, 12 Dec 2011 11:22:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ra8da-000544-Fi for qemu-devel@nongnu.org; Mon, 12 Dec 2011 11:22:00 -0500 Received: from mail-qw0-f45.google.com ([209.85.216.45]:63024) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ra8da-00053x-Ci for qemu-devel@nongnu.org; Mon, 12 Dec 2011 11:21:58 -0500 Received: by qadc10 with SMTP id c10so748960qad.4 for ; Mon, 12 Dec 2011 08:21:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=AI85wN+Cxu0KJWfkJ8Ljm6pwBX//bLCfv8Y/hzgMW8U=; b=noFTSbFU4s6LngfY3KF2wKmgQHPkElgK36TwH2myqK+eulfOCPNL4Uizmvrwuk5rTR 7sJGGX3VH/LQ9HtVnhW0CHdoYm3CTs+6QqP6qv5J/hDicjrpf6jswv6Sptdzsv5JuRoV y+IT2UhXk34E7Ok2dvE3P0/b8OLZuLKBTwX9k= Received: by 10.50.57.138 with SMTP id i10mr15840600igq.3.1323706917686; Mon, 12 Dec 2011 08:21:57 -0800 (PST) Received: from localhost.localdomain (93-34-178-147.ip50.fastwebnet.it. [93.34.178.147]) by mx.google.com with ESMTPS id k39sm1054438ibb.2.2011.12.12.08.21.54 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 12 Dec 2011 08:21:56 -0800 (PST) From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Mon, 12 Dec 2011 17:21:33 +0100 Message-Id: <1323706894-29959-4-git-send-email-pbonzini@redhat.com> X-Mailer: git-send-email 1.7.7.1 In-Reply-To: <1323706894-29959-1-git-send-email-pbonzini@redhat.com> References: <1323706894-29959-1-git-send-email-pbonzini@redhat.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.216.45 Cc: jan.kiszka@siemens.com, alevy@redhat.com Subject: [Qemu-devel] [PATCH v2 3/4] qemu-thread: implement joinable threads for Win32 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Rewrite the handshaking between qemu_thread_create and the win32_start_routine, so that the thread can be joined without races. Similar handshaking is done now between qemu_thread_exit and qemu_thread_join. This also simplifies how QemuThreads are initialized. Signed-off-by: Paolo Bonzini --- qemu-thread-win32.c | 107 +++++++++++++++++++++++++++++++++------------------ qemu-thread-win32.h | 5 +- 3 files changed, 73 insertions(+), 41 deletions(-) diff --git a/qemu-thread-win32.c b/qemu-thread-win32.c index ff80e84..a13ffcc 100644 --- a/qemu-thread-win32.c +++ b/qemu-thread-win32.c @@ -193,41 +193,78 @@ void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex) } struct QemuThreadData { - QemuThread *thread; - void *(*start_routine)(void *); - void *arg; + /* Passed to win32_start_routine. */ + void *(*start_routine)(void *); + void *arg; + short mode; + + /* Only used for joinable threads. */ + bool exited; + void *ret; + CRITICAL_SECTION cs; }; static int qemu_thread_tls_index = TLS_OUT_OF_INDEXES; static unsigned __stdcall win32_start_routine(void *arg) { - struct QemuThreadData data = *(struct QemuThreadData *) arg; - QemuThread *thread = data.thread; - - free(arg); - TlsSetValue(qemu_thread_tls_index, thread); - - /* - * Use DuplicateHandle instead of assigning thread->thread in the - * creating thread to avoid races. It's simpler this way than with - * synchronization. - */ - DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), - GetCurrentProcess(), &thread->thread, - 0, FALSE, DUPLICATE_SAME_ACCESS); - - qemu_thread_exit(data.start_routine(data.arg)); + QemuThreadData *data = (QemuThreadData *) arg; + void *(*start_routine)(void *) = data->start_routine; + void *thread_arg = data->arg; + + if (data->mode == QEMU_THREAD_DETACHED) { + g_free(data); + data = NULL; + } else { + InitializeCriticalSection(&data->cs); + } + TlsSetValue(qemu_thread_tls_index, data); + qemu_thread_exit(start_routine(thread_arg)); abort(); } void qemu_thread_exit(void *arg) { - QemuThread *thread = TlsGetValue(qemu_thread_tls_index); - thread->ret = arg; - CloseHandle(thread->thread); - thread->thread = NULL; - ExitThread(0); + QemuThreadData *data = TlsGetValue(qemu_thread_tls_index); + if (data) { + data->ret = arg; + EnterCriticalSection(&data->cs); + data->exited = true; + LeaveCriticalSection(&data->cs); + } + _endthreadex(0); +} + +void *qemu_thread_join(QemuThread *thread) +{ + QemuThreadData *data; + void *ret; + HANDLE handle; + + data = thread->data; + if (!data) { + return NULL; + } + /* + * Because multiple copies of the QemuThread can exist via + * qemu_thread_get_self, we need to store a value that cannot + * leak there. The simplest, non racy way is to store the TID, + * discard the handle that _beginthreadex gives back, and + * get another copy of the handle here. + */ + EnterCriticalSection(&data->cs); + if (!data->exited) { + handle = OpenThread(SYNCHRONIZE, FALSE, thread->tid); + LeaveCriticalSection(&data->cs); + WaitForSingleObject(handle, INFINITE); + CloseHandle(handle); + } else { + LeaveCriticalSection(&data->cs); + } + ret = data->ret; + DeleteCriticalSection(&data->cs); + g_free(data); + return ret; } static inline void qemu_thread_init(void) @@ -247,37 +284,31 @@ void qemu_thread_create(QemuThread *thread, { HANDLE hThread; - assert(mode == QEMU_THREAD_DETACHED); - struct QemuThreadData *data; qemu_thread_init(); data = g_malloc(sizeof *data); - data->thread = thread; data->start_routine = start_routine; data->arg = arg; + data->mode = mode; + data->exited = false; hThread = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine, - data, 0, NULL); + data, 0, &thread->tid); if (!hThread) { error_exit(GetLastError(), __func__); } CloseHandle(hThread); + thread->data = (mode == QEMU_THREAD_DETACHED) ? NULL : data; } void qemu_thread_get_self(QemuThread *thread) { - if (!thread->thread) { - /* In the main thread of the process. Initialize the QemuThread - pointer in TLS, and use the dummy GetCurrentThread handle as - the identifier for qemu_thread_is_self. */ - qemu_thread_init(); - TlsSetValue(qemu_thread_tls_index, thread); - thread->thread = GetCurrentThread(); - } + qemu_thread_init(); + thread->data = TlsGetValue(qemu_thread_tls_index); + thread->tid = GetCurrentThreadId(); } int qemu_thread_is_self(QemuThread *thread) { - QemuThread *this_thread = TlsGetValue(qemu_thread_tls_index); - return this_thread->thread == thread->thread; + return GetCurrentThreadId() == thread->tid; } diff --git a/qemu-thread-win32.h b/qemu-thread-win32.h index 878f86a..2983490 100644 --- a/qemu-thread-win32.h +++ b/qemu-thread-win32.h @@ -13,9 +13,10 @@ struct QemuCond { HANDLE continue_event; }; +typedef struct QemuThreadData QemuThreadData; struct QemuThread { - HANDLE thread; - void *ret; + QemuThreadData *data; + unsigned tid; }; #endif