From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id AQ7rKlVCs2ktqSYAWB0awg (envelope-from ) for ; Thu, 12 Mar 2026 18:46:45 -0400 Authentication-Results: simark.ca; dkim=fail reason="signature verification failed" (768-bit key; unprotected) header.d=tromey.com header.i=@tromey.com header.a=rsa-sha256 header.s=default header.b=VrJ8J86y; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 9BA521E089; Thu, 12 Mar 2026 18:46:45 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_INVALID,DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_BL_SPAMCOP_NET, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=no autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 86DB61E089 for ; Thu, 12 Mar 2026 18:46:44 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id C75D74B358AB for ; Thu, 12 Mar 2026 22:46:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C75D74B358AB Authentication-Results: sourceware.org; dkim=fail reason="signature verification failed" (768-bit key, unprotected) header.d=tromey.com header.i=@tromey.com header.a=rsa-sha256 header.s=default header.b=VrJ8J86y Received: from omta40.uswest2.a.cloudfilter.net (omta40.uswest2.a.cloudfilter.net [35.89.44.39]) by sourceware.org (Postfix) with ESMTPS id 5D9A14BCA422 for ; Thu, 12 Mar 2026 22:46:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5D9A14BCA422 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5D9A14BCA422 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=35.89.44.39 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1773355575; cv=none; b=pjq/A9kglKRKUS2Ae7WerPxkq8i59hB4XQyYAKO06hPHeQy4Z0Y6TUcaXYDkztNo+8ZuSoQpfHjL/sMG2ArN3asHrWc2jMqcAdIptgd4Ju5ICRLUe4/JY5oKxI/f81GtkQtVJw0Jca1wxAQrnYBiUtkFyrozh+uizi6CiybwHhc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1773355575; c=relaxed/simple; bh=zZEQwDvG/ij7RXQZe2YUHvbo9Re5C+DajKznwFWzPjI=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=DD4Az7kF1AsEhUQ7KNTH0jZ9OVUbnpkyHCiKYUsNppsB9fZ3prByWXHz9ZRH8zt3iQJrHoIdGgurDdDpxFFqU1k2Qf2ngD+2UZPzVaKO0tySTFg7hVMtKJS5Ci/LS0c0xfqkEYbmZsM6HP7PcPhyqgvJSR3N5DqOJresC6ShX0Y= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5D9A14BCA422 Received: from eig-obgw-5002b.ext.cloudfilter.net ([10.0.29.226]) by cmsmtp with ESMTPS id 0hWAwwbYcaPqL0onGw6zlM; Thu, 12 Mar 2026 22:46:14 +0000 Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTPS id 0onDwyus2N3K10onDwbFnP; Thu, 12 Mar 2026 22:46:11 +0000 X-Authority-Analysis: v=2.4 cv=UdRRSLSN c=1 sm=1 tr=0 ts=69b34236 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=Yq5XynenixoA:10 a=ItBw4LHWJt0A:10 a=6tH3jidpuA3dAMx-NMgA:9 a=DCx65vhANUyCzuf5D8fC:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Transfer-Encoding:MIME-Version:Message-ID:Date:Subject: Cc:To:From:Sender:Reply-To:Content-Type:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=8AVHZl/Xk8dq7jKS26jte7+6WDkP7snusrGJW3Bch+A=; b=VrJ8J86yuDWxDo/nIL2vO5hF4x Hv2W5jeYyRf9MzTFvvnlFcWwya+/XVhqC9D5OQPUB24veAtRpKeUcRaahxze6eruTTfFRxZ5a4d2E K923j1JRs/6HmTCrsJQt8PkQ4; Received: from 75-166-225-82.hlrn.qwest.net ([75.166.225.82]:33134 helo=localhost.localdomain) by box5379.bluehost.com with esmtpsa (TLS1.3) tls TLS_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1w0onD-00000001mQl-02Vi; Thu, 12 Mar 2026 16:46:11 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH] Add lock_guard to thread_pool::thread_count Date: Thu, 12 Mar 2026 16:46:04 -0600 Message-ID: <20260312224604.1070829-1-tom@tromey.com> X-Mailer: git-send-email 2.49.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 75.166.225.82 X-Source-L: No X-Exim-ID: 1w0onD-00000001mQl-02Vi X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 75-166-225-82.hlrn.qwest.net (localhost.localdomain) [75.166.225.82]:33134 X-Source-Auth: tom+tromey.com X-Email-Count: 1 X-Org: HG=bhshared;ORG=bluehost; X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-CMAE-Envelope: MS4xfMg4sBrv+6KakjcmJhI+9yZ82gbNSBL+OCa+9Hj+e8QB8ZhTZmOH8Xg/KotBvQSX0afrPVkRqe64GEtGpBzrbCmAj6a13cS5NWBjDDCL01OIHiZ43W0i 9HhLU8JCNomb8DenB/XVNfvhR/XTI4+KCamOL/ODBnKsUaoAFqQHOMzD0IVA04DYmeC+5tgBuKgnDcOr0TDueSOyeYUTmAqeueQ= X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org I think there's a possible race when calling thread_pool::thread_count from a worker thread, if the main thread changes the number of threads at the same time. When this code was initially written, we didn't worry about this, because this was only accessed from the main thread. This is no longer the case, though. This patch fixes any potential problem by adding a lock_guard to the method. This doesn't seem too harmful because this is not called very much and because I doubt this lock is highly contended. While doing this I noticed that thread_pool::do_post_task could access m_sized_at_least_once and m_thread_count without holding the lock. This patch changes this method as well. --- gdbsupport/thread-pool.cc | 47 +++++++++++++++++++++++++++++---------- gdbsupport/thread-pool.h | 9 +------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/gdbsupport/thread-pool.cc b/gdbsupport/thread-pool.cc index 6940773d58a..4dc72e56642 100644 --- a/gdbsupport/thread-pool.cc +++ b/gdbsupport/thread-pool.cc @@ -148,6 +148,17 @@ thread_pool::~thread_pool () case -- see the comment by the definition of g_thread_pool. */ } +size_t +thread_pool::thread_count () +{ +#if CXX_STD_THREAD + std::lock_guard guard (m_tasks_mutex); + return m_thread_count; +#else + return 0; +#endif +} + void thread_pool::set_thread_count (size_t num_threads) { @@ -197,21 +208,33 @@ thread_pool::set_thread_count (size_t num_threads) void thread_pool::do_post_task (std::packaged_task &&func) { - /* This assert is here to check that no tasks are posted to the pool between - its initialization and sizing. */ - gdb_assert (m_sized_at_least_once); - std::packaged_task t (std::move (func)); + /* This is a bit strange but we don't want to hold the lock when + calling FUNC in the case where it must be called immediately. + Although that seems pathological, there are some weird cases (a + moribund worker thread or FUNC itself submitting a task) and it + seemed best to be safe. */ + bool call_immediately = false; - if (m_thread_count != 0) - { - std::lock_guard guard (m_tasks_mutex); - m_tasks.emplace (std::move (t)); - m_tasks_cv.notify_one (); - } - else + { + std::lock_guard guard (m_tasks_mutex); + + /* This assert is here to check that no tasks are posted to the + pool between its initialization and sizing. */ + gdb_assert (m_sized_at_least_once); + + if (m_thread_count != 0) + { + m_tasks.emplace (std::move (func)); + m_tasks_cv.notify_one (); + } + else + call_immediately = true; + } + + if (call_immediately) { /* Just execute it now. */ - t (); + func (); } } diff --git a/gdbsupport/thread-pool.h b/gdbsupport/thread-pool.h index 50b7a064d7e..a9188d1e107 100644 --- a/gdbsupport/thread-pool.h +++ b/gdbsupport/thread-pool.h @@ -50,14 +50,7 @@ class thread_pool void set_thread_count (size_t num_threads); /* Return the number of executing threads. */ - size_t thread_count () const - { -#if CXX_STD_THREAD - return m_thread_count; -#else - return 0; -#endif - } + size_t thread_count (); /* Post a task to the thread pool. A future is returned, which can be used to wait for the result. */ -- 2.49.0