fuzz: Remove unnecessary thread pool mutexes
What the developer wrote
Authored by marcofleon
fuzz: Remove unnecessary thread pool mutexes
Remove the `Mutex` from the `threadpool` and `dbwrapper_concurrent_reads`
pool startup helpers. Fuzz targets are entered sequentially within a
process and parallel fuzzing uses separate processes/forks, which each
have their own copy of the global thread pool. Therefore, a mutex to
prevent two in-process callers from racing to start the pool isn't needed.
What changed, and why it matters
This commit removes mutexes (thread-safety locks) from two internal Bitcoin Core fuzz-test helpers. Fuzz tests are automated test harnesses, not production code, and the commit explains that the locks were unnecessary because fuzz targets run sequentially within a single process. There is no indication this change affects real Bitcoin Core users or introduces a security vulnerability.
No action required. Treat as a code-cleanup/test-maintenance change. Standard review/CI verification is sufficient.
Security signals we found
Change is confined to fuzz-test harness code (src/test/fuzz/*)
Removal of synchronization primitives in non-production test code
Commit message explicitly states the mutexes were unnecessary for the fuzzing execution model
Evidence from the diff
The patch deletes Mutex g_read_pool_mutex and Mutex g_pool_mutex from dbwrapper.cpp and threadpool.cpp fuzz targets, along with their LOCK() calls and EXCLUSIVE_LOCKS_REQUIRED annotations. The reasoning given is that fuzz targets are entered sequentially within a process and parallel fuzzing uses separate processes/forks, each with their own global thread pool instance, so in-process mutual exclusion is not required. The change is purely in test/fuzz code and does not alter consensus, networking, wallet, or node runtime behavior.
Changed components
src/test/fuzz/dbwrapper.cppsrc/test/fuzz/threadpool.cppInspect captured patch +4 / −8
diff --git a/src/test/fuzz/dbwrapper.cpp b/src/test/fuzz/dbwrapper.cpp
index 8f1d84e4..265f0ca3 100644
--- a/src/test/fuzz/dbwrapper.cpp
+++ b/src/test/fuzz/dbwrapper.cpp
@@ -174,11 +174,9 @@ constexpr size_t MAX_READ_WORKERS{8};
constexpr size_t MAX_READ_QUERIES_PER_WORKER{128};
ThreadPool g_read_pool{"dbfuzz"};
-Mutex g_read_pool_mutex;
-void StartReadPoolIfNeeded() EXCLUSIVE_LOCKS_REQUIRED(!g_read_pool_mutex)
+void StartReadPoolIfNeeded()
{
- LOCK(g_read_pool_mutex);
if (!g_read_pool.WorkersCount()) g_read_pool.Start(MAX_READ_WORKERS);
}
@@ -361,7 +359,7 @@ FUZZ_TARGET(dbwrapper_threaded, .init = [] { static auto setup{MakeNoLogFileCont
/*allow_force_compact=*/true);
}
-FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLogFileContext<>()}; }) EXCLUSIVE_LOCKS_REQUIRED(!g_read_pool_mutex)
+FUZZ_TARGET(dbwrapper_concurrent_reads, .init = [] { static auto setup{MakeNoLogFileContext<>()}; })
{
StartReadPoolIfNeeded();
SeedRandomStateForTest(SeedRand::ZEROS);
diff --git a/src/test/fuzz/threadpool.cpp b/src/test/fuzz/threadpool.cpp
index a5b01db1..48ec3ec8 100644
--- a/src/test/fuzz/threadpool.cpp
+++ b/src/test/fuzz/threadpool.cpp
@@ -43,13 +43,11 @@ static void GetFuture(std::future<void>& future, uint32_t& fail_counter)
// instability in the fuzzing environment.
// This is also how we use it in the app's lifecycle.
ThreadPool g_pool{"fuzz"};
-Mutex g_pool_mutex;
// Global to verify we always have the same number of threads.
size_t g_num_workers = 3;
-static void StartPoolIfNeeded() EXCLUSIVE_LOCKS_REQUIRED(!g_pool_mutex)
+static void StartPoolIfNeeded()
{
- LOCK(g_pool_mutex);
if (g_pool.WorkersCount() == g_num_workers) return;
g_pool.Start(g_num_workers);
}
@@ -60,7 +58,7 @@ static void setup_threadpool_test()
LogInstance().DisableLogging();
}
-FUZZ_TARGET(threadpool, .init = setup_threadpool_test) EXCLUSIVE_LOCKS_REQUIRED(!g_pool_mutex)
+FUZZ_TARGET(threadpool, .init = setup_threadpool_test)
{
// Because LibAFL calls fork() after calling the init setup function,
// the child processes end up having one thread active and no workers.
Why this scored 13/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.