From nobody Mon Apr 29 20:00:34 2024 Delivered-To: importer@patchew.org Received-SPF: temperror (zoho.com: Error in retrieving data from DNS) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=temperror (zoho.com: Error in retrieving data from DNS) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511833951899389.1737839370036; Mon, 27 Nov 2017 17:52:31 -0800 (PST) Received: from localhost ([::1]:35521 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJV4F-0003G0-6i for importer@patchew.org; Mon, 27 Nov 2017 20:52:11 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59302) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJV36-0002gd-A2 for qemu-devel@nongnu.org; Mon, 27 Nov 2017 20:51:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJV33-0003ax-7I for qemu-devel@nongnu.org; Mon, 27 Nov 2017 20:51:00 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:2049 helo=huawei.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJV32-0003Ym-Kh for qemu-devel@nongnu.org; Mon, 27 Nov 2017 20:50:57 -0500 Received: from DGGEMS401-HUB.china.huawei.com (unknown [172.30.72.59]) by Forcepoint Email with ESMTP id EB74DF1415A89; Tue, 28 Nov 2017 09:50:49 +0800 (CST) Received: from localhost (10.177.131.80) by DGGEMS401-HUB.china.huawei.com (10.3.19.201) with Microsoft SMTP Server id 14.3.361.1; Tue, 28 Nov 2017 09:50:43 +0800 From: linzhecheng To: Date: Tue, 28 Nov 2017 09:50:39 +0800 Message-ID: <20171128015039.19060-1-linzhecheng@huawei.com> X-Mailer: git-send-email 2.12.2.windows.2 MIME-Version: 1.0 X-Originating-IP: [10.177.131.80] X-CFilter-Loop: Reflected X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 45.249.212.191 Subject: [Qemu-devel] [PATCH v2] thread: move detach_thread from creating thread to created thread X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pbonzini@redhat.com, aliguori@us.ibm.com, arei.gonglei@huawei.com, famz@redhat.com, linzhecheng Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_6 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" If we create a thread with QEMU_THREAD_DETACHED mode, QEMU may get a segfau= lt in a low probability. The backtrace is: arg=3Darg@entry=3D0x3f5cf70, mode=3Dmode@entry=3D1) at util/qemu_thread= _posix.c:512 at io/task.c:141 destroy=3Ddestroy@entry=3D0x0) at io/channel_socket.c:194 The root cause of this problem is a bug of glibc(version 2.17,the latest ve= rsion have the same bug), let's see what happened in glibc's code. Here is the code slice of pthread_detach.c 25 int 26 pthread_detach (pthread_t th) 27 { 28 struct pthread *pd =3D (struct pthread *) th; 29 30 /* Make sure the descriptor is valid. */ 31 if (INVALID_NOT_TERMINATED_TD_P (pd)) 32 /* Not a valid thread handle. */ 34 return ESRCH; 35 36 int result =3D 0; 37 /* Mark the thread as detached. */ 38 if (atomic_compare_and_exchange_bool_acq (&pd->joinid, pd, NULL)) 39 { 40 /* There are two possibilities here. First, the thread might 41 already be detached. In this case we return EINVAL. 42 Otherwise there might already be a waiter. The standard does 43 not mention what happens in this case. */ 44 if (IS_DETACHED (pd)) 45 result =3D EINVAL; 46 } 47 else 48 /* Check whether the thread terminated meanwhile. In this case we 49 will just free the TCB. */ 50 if ((pd->cancelhandling & EXITING_BITMASK) !=3D 0) 51 /* Note that the code in __free_tcb makes sure each thread 52 control block is freed only once. */ 53 __free_tcb (pd); 54 return result; 55} QEMU get a segfault at line 50, becasue pd is an invalid address. pd is still valid at line 38 when set pd->joinid =3D pd, at this moment, created thread is just exiting(only keeps runing for a short time), created thread is running in code of start_thread: 404 /* If the thread is detached free the TCB. */ 405 if (IS_DETACHED (pd)) 406 /* Free the TCB. */ 407 __free_tcb (pd); created thread found that pd is detached, so it freed pd, in this case, pd became an invalid address. I rewrite qemu_thread_create to move detach_thread from creating thread to = created to avoid this concurrency problem. Change-Id: I2293d5be1526241cf58785d701b922f2ffc6491b Signed-off-by: linzhecheng Reviewed-by: Fam Zheng --- include/qemu/thread-posix.h | 8 ++++++++ include/qemu/thread.h | 1 + util/qemu-thread-posix.c | 45 ++++++++++++++++++++++++++++++++++-------= ---- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h index f3f47e426f..d855c15dab 100644 --- a/include/qemu/thread-posix.h +++ b/include/qemu/thread-posix.h @@ -44,4 +44,12 @@ struct QemuThread { pthread_t thread; }; =20 +struct QemuThread_args { + void *(*start_routine)(void *); + void *arg; + char *name; + int mode; +}; + + #endif diff --git a/include/qemu/thread.h b/include/qemu/thread.h index 9910f49b3a..db365242da 100644 --- a/include/qemu/thread.h +++ b/include/qemu/thread.h @@ -10,6 +10,7 @@ typedef struct QemuSemaphore QemuSemaphore; typedef struct QemuEvent QemuEvent; typedef struct QemuLockCnt QemuLockCnt; typedef struct QemuThread QemuThread; +typedef struct QemuThread_args QemuThread_args; =20 #ifdef _WIN32 #include "qemu/thread-win32.h" diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c index 7306475899..07b5838862 100644 --- a/util/qemu-thread-posix.c +++ b/util/qemu-thread-posix.c @@ -489,6 +489,30 @@ static void qemu_thread_set_name(QemuThread *thread, c= onst char *name) #endif } =20 +static void *qemu_thread_start(void *args) +{ + QemuThread_args *qemu_thread_args; + void *ret; + QemuThread qemu_thread; + + qemu_thread_args =3D (QemuThread_args *)args; + qemu_thread_get_self(&qemu_thread); + + if (qemu_thread_args->name) { + qemu_thread_set_name(&qemu_thread, qemu_thread_args->name); + g_free(qemu_thread_args->name); + } + + if (qemu_thread_args->mode =3D=3D QEMU_THREAD_DETACHED) { + pthread_detach(qemu_thread.thread); + } + ret =3D qemu_thread_args->start_routine(qemu_thread_args->arg); + + g_free(qemu_thread_args); + return ret; +} + + void qemu_thread_create(QemuThread *thread, const char *name, void *(*start_routine)(void*), void *arg, int mode) @@ -496,6 +520,7 @@ void qemu_thread_create(QemuThread *thread, const char = *name, sigset_t set, oldset; int err; pthread_attr_t attr; + QemuThread_args *qemu_thread_args; =20 err =3D pthread_attr_init(&attr); if (err) { @@ -505,20 +530,18 @@ void qemu_thread_create(QemuThread *thread, const cha= r *name, /* Leave signal handling to the iothread. */ sigfillset(&set); pthread_sigmask(SIG_SETMASK, &set, &oldset); - err =3D pthread_create(&thread->thread, &attr, start_routine, arg); + + qemu_thread_args =3D g_new0(QemuThread_args, 1); + qemu_thread_args->mode =3D mode; + qemu_thread_args->name =3D name_threads ? g_strdup_printf("%s", name) = : NULL; + qemu_thread_args->start_routine =3D start_routine; + qemu_thread_args->arg =3D arg; + + err =3D pthread_create(&thread->thread, &attr, + qemu_thread_start, qemu_thread_args); if (err) error_exit(err, __func__); =20 - if (name_threads) { - qemu_thread_set_name(thread, name); - } - - if (mode =3D=3D QEMU_THREAD_DETACHED) { - err =3D pthread_detach(thread->thread); - if (err) { - error_exit(err, __func__); - } - } pthread_sigmask(SIG_SETMASK, &oldset, NULL); =20 pthread_attr_destroy(&attr); --=20 2.12.2.windows.2