From patchwork Mon Nov 5 13:55:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 10668323 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 649AD14BD for ; Mon, 5 Nov 2018 13:58:58 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5478D29517 for ; Mon, 5 Nov 2018 13:58:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 48A4C29521; Mon, 5 Nov 2018 13:58:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,SUBJ_OBFU_PUNCT_FEW autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B6C1029517 for ; Mon, 5 Nov 2018 13:58:57 +0000 (UTC) Received: from localhost ([::1]:35407 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfP6-0005Bn-Da for patchwork-qemu-devel@patchwork.kernel.org; Mon, 05 Nov 2018 08:58:56 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55146) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJfMC-0001I6-Oe for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gJfMB-0006pL-Rt for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:56 -0500 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:52278) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gJfMB-0006eU-Ie for qemu-devel@nongnu.org; Mon, 05 Nov 2018 08:55:55 -0500 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1gJfLy-0006HX-E0; Mon, 05 Nov 2018 13:55:42 +0000 From: Peter Maydell To: qemu-devel@nongnu.org Date: Mon, 5 Nov 2018 13:55:36 +0000 Message-Id: <20181105135538.28025-1-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.1 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH for-3.1 0/2] Fix qemu_thread_atexit* for OSX 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: Kevin Wolf , Paolo Bonzini , patches@linaro.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Our current implementation of qemu_thread_atexit* is broken on OSX. This is because it works by cerating a piece of thread-specific data with pthread_key_create() and using the destructor function for that data to run the notifier function passed to it by the caller of qemu_thread_atexit_add(). The expected use case is that the caller uses a __thread variable as the notifier, and uses the callback to clean up information that it is keeping per-thread in __thread variables. Unfortunately, on OSX this does not work, because on OSX a __thread variable may be destroyed (freed) before the pthread_key_create() destructor runs. (POSIX imposes no ordering constraint here; the OSX implementation happens to implement __thread variables in terms of pthread_key_create((), whereas Linux uses different mechanisms that mean the __thread variables will still be present when the pthread_key_create() destructor is run.) Fix this by switching to a scheme similar to the one qemu-thread-win32 uses for qemu_thread_atexit: keep the thread's notifiers on a __thread variable, and run the notifiers on calls to qemu_thread_exit() and on return from the start routine passed to qemu_thread_start(). We do this with the pthread_cleanup_push() API. (This approach was suggested by Paolo.) There is a slightly awkward corner here for the main thread, which isn't started via qemu_thread_start() and so can exit without passing through the cleanup handler. qemu-thread-win32.c tries to handle the main thread using an atexit() handler to run its notifiers. I don't think this will work because the atexit handler may not run on the main thread, in which case notify callback functions which refer to __thread variables will get the wrong ones. So instead I have documented that the API leaves it unspecified whether notifiers are run for thread exits that happen when the entire process is exiting. This is OK for our current users (who only use the callbacks to clean up memory or win32 Fibers which are deleted on process exit anyway), and means we don't need to worry about running callbacks on main thread exit (which always causes the whole process to exit). In particular, this fixes the ASAN errors and lockups we currently see on OSX hosts when running test-bdrv-drain. thanks -- PMM Peter Maydell (2): include/qemu/thread.h: Document qemu_thread_atexit* API util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX include/qemu/thread.h | 22 ++++++++++++++++++++ util/qemu-thread-posix.c | 44 ++++++++++++++++++---------------------- 2 files changed, 42 insertions(+), 24 deletions(-)