From nobody Mon Feb 9 02:50:53 2026 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 1509961739233958.926847784457; Mon, 6 Nov 2017 01:48:59 -0800 (PST) Received: from localhost ([::1]:47178 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBe1L-0004Ib-9n for importer@patchew.org; Mon, 06 Nov 2017 04:48:43 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47498) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eBdzp-0003dX-B0 for qemu-devel@nongnu.org; Mon, 06 Nov 2017 04:47:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eBdzo-0003aB-AN for qemu-devel@nongnu.org; Mon, 06 Nov 2017 04:47:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35964) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eBdzo-0003a1-1t for qemu-devel@nongnu.org; Mon, 06 Nov 2017 04:47:08 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F2E85285B1; Mon, 6 Nov 2017 09:47:06 +0000 (UTC) Received: from pxdev.xzpeter.org.com (ovpn-12-165.pek2.redhat.com [10.72.12.165]) by smtp.corp.redhat.com (Postfix) with ESMTP id 66BA66A026; Mon, 6 Nov 2017 09:47:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F2E85285B1 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=peterx@redhat.com From: Peter Xu To: qemu-devel@nongnu.org Date: Mon, 6 Nov 2017 17:46:17 +0800 Message-Id: <20171106094643.14881-2-peterx@redhat.com> In-Reply-To: <20171106094643.14881-1-peterx@redhat.com> References: <20171106094643.14881-1-peterx@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 06 Nov 2017 09:47:07 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [RFC v3 01/27] char-io: fix possible race on IOWatchPoll 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: Laurent Vivier , Fam Zheng , Juan Quintela , mdroth@linux.vnet.ibm.com, peterx@redhat.com, Markus Armbruster , marcandre.lureau@redhat.com, Stefan Hajnoczi , Paolo Bonzini , Jiri Denemark , "Dr . David Alan Gilbert" Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_6 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This is not a problem if we are only having one single loop thread like before. However, after per-monitor thread is introduced, this is not true any more, and the race can happen. The race can be triggered with "make check -j8" sometimes: qemu-system-x86_64: /root/git/qemu/chardev/char-io.c:91: io_watch_poll_finalize: Assertion `iwp->src =3D=3D NULL' failed. This patch keeps the reference for the watch object when creating in io_add_watch_poll(), so that the object will never be released in the context main loop, especially when the context loop is running in another standalone thread. Meanwhile, when we want to remove the watch object, we always first detach the watch object from its owner context, then we continue with the cleanup. Without this patch, calling io_remove_watch_poll() in main loop thread is not thread-safe, since the other per-monitor thread may be modifying the watch object at the same time. Reviewed-by: Marc-Andr=C3=A9 Lureau Signed-off-by: Peter Xu Reviewed-by: Fam Zheng --- chardev/char-io.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/chardev/char-io.c b/chardev/char-io.c index f81052481a..50b5bac704 100644 --- a/chardev/char-io.c +++ b/chardev/char-io.c @@ -122,7 +122,6 @@ GSource *io_add_watch_poll(Chardev *chr, g_free(name); =20 g_source_attach(&iwp->parent, context); - g_source_unref(&iwp->parent); return (GSource *)iwp; } =20 @@ -131,12 +130,25 @@ static void io_remove_watch_poll(GSource *source) IOWatchPoll *iwp; =20 iwp =3D io_watch_poll_from_source(source); + + /* + * Here the order of destruction really matters. We need to first + * detach the IOWatchPoll object from the context (which may still + * be running in another loop thread), only after that could we + * continue to operate on iwp->src, or there may be race condition + * between current thread and the context loop thread. + * + * Let's blame the glib bug mentioned in commit 2b316774f6 + * ("qemu-char: do not operate on sources from finalize + * callbacks") for this extra complexity. + */ + g_source_destroy(&iwp->parent); if (iwp->src) { g_source_destroy(iwp->src); g_source_unref(iwp->src); iwp->src =3D NULL; } - g_source_destroy(&iwp->parent); + g_source_unref(&iwp->parent); } =20 void remove_fd_in_watch(Chardev *chr) --=20 2.13.5