All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: linzhecheng <linzhecheng@huawei.com>
Subject: [Qemu-devel] [PULL 02/46] qemu-thread: fix races on threads that exit very quickly
Date: Wed, 20 Dec 2017 18:14:14 +0100	[thread overview]
Message-ID: <1513790098-9815-3-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1513790098-9815-1-git-send-email-pbonzini@redhat.com>

From: linzhecheng <linzhecheng@huawei.com>

If we create a thread with QEMU_THREAD_DETACHED mode, QEMU may get a segfault with low probability.

The backtrace is:
   #0  0x00007f46c60291d7 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
   #1  0x00007f46c602a8c8 in __GI_abort () at abort.c:90
   #2  0x00000000008543c9 in PAT_abort ()
   #3  0x000000000085140d in patchIllInsHandler ()
   #4  <signal handler called>
   #5  pthread_detach (th=139933037614848) at pthread_detach.c:50
   #6  0x0000000000829759 in qemu_thread_create (thread=thread@entry=0x7ffdaa8205e0, name=name@entry=0x94d94a "io-task-worker", start_routine=start_routine@entry=0x7eb9a0 <qio_task_thread_worker>,
       arg=arg@entry=0x3f5cf70, mode=mode@entry=1) at util/qemu_thread_posix.c:512
   #7  0x00000000007ebc96 in qio_task_run_in_thread (task=0x31db2c0, worker=worker@entry=0x7e7e40 <qio_channel_socket_connect_worker>, opaque=0xcd23380, destroy=0x7f1180 <qapi_free_SocketAddress>)
       at io/task.c:141
   #8  0x00000000007e7f33 in qio_channel_socket_connect_async (ioc=ioc@entry=0x626c0b0, addr=<optimized out>, callback=callback@entry=0x55e080 <qemu_chr_socket_connected>, opaque=opaque@entry=0x42862c0,
       destroy=destroy@entry=0x0) at io/channel_socket.c:194
   #9  0x000000000055bdd1 in socket_reconnect_timeout (opaque=0x42862c0) at qemu_char.c:4744
   #10 0x00007f46c72483b3 in g_timeout_dispatch () from /usr/lib64/libglib-2.0.so.0
   #11 0x00007f46c724799a in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
   #12 0x000000000076c646 in glib_pollfds_poll () at main_loop.c:228
   #13 0x000000000076c6eb in os_host_main_loop_wait (timeout=348000000) at main_loop.c:273
   #14 0x000000000076c815 in main_loop_wait (nonblocking=nonblocking@entry=0) at main_loop.c:521
   #15 0x000000000056a511 in main_loop () at vl.c:2076
   #16 0x0000000000420705 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4940

The cause of this problem is a glibc bug; for more information, see
https://sourceware.org/bugzilla/show_bug.cgi?id=19951.
The solution for this bug is to use pthread_attr_setdetachstate.

There is a similar issue with pthread_setname_np, which is moved
from creating thread to created thread.

Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Message-Id: <20171128044656.10592-1-linzhecheng@huawei.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
[Simplify the code by removing qemu_thread_set_name, and free the arguments
 before invoking the start routine. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/qemu-thread-posix.c | 59 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 42 insertions(+), 17 deletions(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 7306475..fcd369b 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -479,15 +479,29 @@ static void __attribute__((constructor)) qemu_thread_atexit_init(void)
 }
 
 
-/* Attempt to set the threads name; note that this is for debug, so
- * we're not going to fail if we can't set it.
- */
-static void qemu_thread_set_name(QemuThread *thread, const char *name)
-{
 #ifdef CONFIG_PTHREAD_SETNAME_NP
-    pthread_setname_np(thread->thread, name);
-#endif
+typedef struct {
+    void *(*start_routine)(void *);
+    void *arg;
+    char *name;
+} QemuThreadArgs;
+
+static void *qemu_thread_start(void *args)
+{
+    QemuThreadArgs *qemu_thread_args = args;
+    void *(*start_routine)(void *) = qemu_thread_args->start_routine;
+    void *arg = qemu_thread_args->arg;
+
+    /* Attempt to set the threads name; note that this is for debug, so
+     * we're not going to fail if we can't set it.
+     */
+    pthread_setname_np(pthread_self(), qemu_thread_args->name);
+    g_free(qemu_thread_args->name);
+    g_free(qemu_thread_args);
+    return start_routine(arg);
 }
+#endif
+
 
 void qemu_thread_create(QemuThread *thread, const char *name,
                        void *(*start_routine)(void*),
@@ -496,29 +510,40 @@ void qemu_thread_create(QemuThread *thread, const char *name,
     sigset_t set, oldset;
     int err;
     pthread_attr_t attr;
+    QemuThreadArgs *qemu_thread_args;
 
     err = pthread_attr_init(&attr);
     if (err) {
         error_exit(err, __func__);
     }
 
+    if (mode == QEMU_THREAD_DETACHED) {
+        pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+    }
+
     /* Leave signal handling to the iothread.  */
     sigfillset(&set);
     pthread_sigmask(SIG_SETMASK, &set, &oldset);
-    err = pthread_create(&thread->thread, &attr, start_routine, arg);
-    if (err)
-        error_exit(err, __func__);
 
+#ifdef CONFIG_PTHREAD_SETNAME_NP
     if (name_threads) {
-        qemu_thread_set_name(thread, name);
+        qemu_thread_args = g_new0(QemuThreadArgs, 1);
+        qemu_thread_args->name = g_strdup(name);
+        qemu_thread_args->start_routine = start_routine;
+        qemu_thread_args->arg = arg;
+
+        err = pthread_create(&thread->thread, &attr,
+                             qemu_thread_start, qemu_thread_args);
+    } else
+#endif
+    {
+        err = pthread_create(&thread->thread, &attr,
+                             start_routine, arg);
     }
 
-    if (mode == QEMU_THREAD_DETACHED) {
-        err = pthread_detach(thread->thread);
-        if (err) {
-            error_exit(err, __func__);
-        }
-    }
+    if (err)
+        error_exit(err, __func__);
+
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 
     pthread_attr_destroy(&attr);
-- 
1.8.3.1

  parent reply	other threads:[~2017-12-20 17:15 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-20 17:14 [Qemu-devel] [PULL 00/46] First batch of misc patches for QEMU 2.12 Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 01/46] memfd: fix configure test Paolo Bonzini
2018-04-30 15:49   ` Greg Kurz
2017-12-20 17:14 ` Paolo Bonzini [this message]
2017-12-20 17:14 ` [Qemu-devel] [PULL 03/46] qemu-pr-helper: miscellaneous fixes Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 04/46] contrib: add systemd unit files Paolo Bonzini
2018-01-10 14:44   ` Daniel P. Berrange
2017-12-20 17:14 ` [Qemu-devel] [PULL 05/46] Revert "docker: Enable features explicitly in test-full" Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 06/46] scsi-block: Add share-rw option Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 07/46] MAITAINERS: List Fam Zheng as reviewer for SCSI patches Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 08/46] x86/cpu: Enable new SSE/AVX/AVX512 cpu features Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 09/46] hyperv: set partition-wide MSRs only on first vcpu Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 10/46] hyperv: ensure SINTx msrs are reset properly Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 11/46] hyperv: make SynIC version msr constant Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 12/46] cpus: make pause_all_cpus() play with SMP on single threaded TCG Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 13/46] cpu-exec: fix missed CPU kick during interrupt injection Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 14/46] target/i386: Fix compiler warnings Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 15/46] baum: Truncate braille device size to 84x1 Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 16/46] sockets: remove obsolete code that updated listen address Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 17/46] target/i386: Fix handling of VEX prefixes Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 18/46] tests/boot-serial-test: Make sure that we check the timeout regularly Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 19/46] tests/boot-serial-test: Add code to allow to specify our own kernel or bios Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 20/46] tests/boot-serial-test: Add support for the mcf5208evb board Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 21/46] tests/boot-serial-test: Add tests for microblaze boards Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 22/46] hw/moxie/moxiesim: Add support for loading a BIOS on moxiesim Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 23/46] tests/boot-serial-test: Add a test for the moxiesim machine Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 24/46] tests/boot-serial-test: Add support for the raspi2 machine Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 25/46] cpu: refactor cpu_address_space_init() Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 26/46] cpu: suffix cpu address spaces with cpu index Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 27/46] block/iscsi: dont leave allocmap in an invalid state on UNMAP failure Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 28/46] block/iscsi: only report an iSCSI Failure if we don't handle it gracefully Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 29/46] exec: Don't reuse unassigned_mem_ops for io_mem_rom Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 30/46] hw/mips/boston: Remove workaround for writes to ROM aborting Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 31/46] hw/i386/vmport: replace fprintf() by trace events or LOG_UNIMP Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 32/46] scsi: provide general-purpose functions to manage sense data Paolo Bonzini
2017-12-22 15:25   ` Roman Kagan
2017-12-20 17:14 ` [Qemu-devel] [PULL 33/46] scsi: replace hex constants with #defines Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 34/46] Remove legacy -no-kvm-pit option Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 35/46] i8259: convert DPRINTFs into trace Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 36/46] i8259: use DEBUG_IRQ_COUNT always Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 37/46] i8259: generalize statistics into common code Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 38/46] kvm-i8259: support "info pic" and "info irq" Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 39/46] i8259: move TYPE_INTERRUPT_STATS_PROVIDER upper Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 40/46] checkpatch: volatile with a comment or sig_atomic_t is okay Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 41/46] rcu: reduce more than 7MB heap memory by malloc_trim() Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 42/46] chardev: fix backend events regression with mux chardev Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 43/46] test: add some chardev mux event tests Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 44/46] blockdev: convert internal NBD server to QIONetListener Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 45/46] blockdev: convert qemu-nbd " Paolo Bonzini
2017-12-20 17:14 ` [Qemu-devel] [PULL 46/46] chardev: convert the socket " Paolo Bonzini
2017-12-20 18:21 ` [Qemu-devel] [PULL 00/46] First batch of misc patches for QEMU 2.12 no-reply
2017-12-20 19:20 ` Peter Maydell
2017-12-20 21:56   ` Paolo Bonzini
2017-12-21 12:49     ` [Qemu-devel] out of bounds in set_cc_op() (was: [PULL 00/46] First batch of misc patches for QEMU 2.12) Thomas Huth
2017-12-21 13:07       ` [Qemu-devel] out of bounds in set_cc_op() Laurent Vivier
2017-12-21 13:32         ` Laurent Vivier
2017-12-21 14:10           ` Paolo Bonzini
2017-12-21 14:13             ` Laurent Vivier
2017-12-21 14:14               ` Paolo Bonzini
2017-12-21 14:36                 ` Laurent Vivier
2017-12-21 19:20                   ` Laurent Vivier
2017-12-21 19:30                     ` Paolo Bonzini
2018-01-02 16:57                       ` Thomas Huth
2017-12-21  3:29 [Qemu-devel] [PULL 02/46] qemu-thread: fix races on threads that exit very quickly linzhecheng
2017-12-21  3:35 ` Eric Blake
2017-12-21  3:40 linzhecheng
2017-12-21 14:33 ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1513790098-9815-3-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=linzhecheng@huawei.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.