[binutils-gdb] Add lock_guard to thread_pool::thread_count

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c3d4baadd233635c13c306d9fdff3342710633ed

commit c3d4baadd233635c13c306d9fdff3342710633ed
Author: Tom Tromey <[email protected]>
Date:   Thu Mar 12 14:00:44 2026 -0600

    Add lock_guard to thread_pool::thread_count
    
    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.
    
    Approved-by: Kevin Buettner <[email protected]>

Diff:
---
 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<std::mutex> 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<void ()> &&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<void ()> 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<std::mutex> guard (m_tasks_mutex);
-      m_tasks.emplace (std::move (t));
-      m_tasks_cv.notify_one ();
-    }
-  else
+  {
+    std::lock_guard<std::mutex> 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 @@ public:
   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.  */