From 95d90a7be515ecb895b2f5a3f265bb2b271a4eca Mon Sep 17 00:00:00 2001
From: Mikulas Patocka <mikulas@twibright.com>
Date: Sun, 6 Apr 2025 19:11:29 +0200
Subject: [PATCH] Fix a race condition in scoped_timer::finalize

scoped_timer::finalize is called from fork. However, it may race with
other threads creating or freeing timer threads.

This patch reworks scoped_timer::finalize so that it runs only once
(there is no need to repeat it). We remove the assignments "num_workers =
0" and "available_workers.clear();" - they were done without any locks,
thus they were subjects to race condition.

The variable num_workers is deleted because it is not needed.

There was another bug in scoped_timer::finalize - if some workers were
busy, the function would spin-wait for them to terminate. This patch
changes it so that busy workers are ignored.

Signed-off-by: Mikulas Patocka <mikulas@twibright.com>
---
 src/util/scoped_timer.cpp | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/src/util/scoped_timer.cpp b/src/util/scoped_timer.cpp
index 4a62b9280..2347132ce 100644
--- a/src/util/scoped_timer.cpp
+++ b/src/util/scoped_timer.cpp
@@ -48,7 +48,6 @@ struct scoped_timer_state {
 
 static std::vector<scoped_timer_state*> available_workers;
 static std::mutex workers;
-static atomic<unsigned> num_workers(0);
 
 static void thread_func(scoped_timer_state *s) {
     workers.lock();
@@ -94,7 +93,6 @@ scoped_timer::scoped_timer(unsigned ms, event_handler * eh) {
         // start new thead
         workers.unlock();
         s = new scoped_timer_state;
-        ++num_workers;
         init_state(ms, eh);
         s->m_thread = std::thread(thread_func, s);
     }
@@ -122,34 +120,27 @@ scoped_timer::~scoped_timer() {
 
 void scoped_timer::initialize() {
 #ifndef _WINDOWS
-    static bool pthread_atfork_set = false;
-    if (!pthread_atfork_set) {
+    static std::atomic<bool> pthread_atfork_set = false;
+    if (!pthread_atfork_set.exchange(true)) {
         pthread_atfork(finalize, nullptr, nullptr);
-        pthread_atfork_set = true;
     }
 #endif
 }
 
 void scoped_timer::finalize() {
-    unsigned deleted = 0;
-    while (deleted < num_workers) {
-        workers.lock();
-        for (auto w : available_workers) {
-            w->work = EXITING;
-            w->cv.notify_one();
-        }
-        decltype(available_workers) cleanup_workers;
-        std::swap(available_workers, cleanup_workers);
-        workers.unlock();
-
-        for (auto w : cleanup_workers) {
-            ++deleted;
-            w->m_thread.join();
-            delete w;
-        }
+    workers.lock();
+    for (auto w : available_workers) {
+        w->work = EXITING;
+        w->cv.notify_one();
+    }
+    decltype(available_workers) cleanup_workers;
+    std::swap(available_workers, cleanup_workers);
+    workers.unlock();
+
+    for (auto w : cleanup_workers) {
+        w->m_thread.join();
+        delete w;
     }
-    num_workers = 0;
-    available_workers.clear();
 }
 
 void scoped_timer::init_state(unsigned ms, event_handler * eh) {