Case Study: The Thread Pool Passed 64/64 Tasks. ThreadSanitizer Found the Race in the Destructor

The thread pool passed every functional test on the first run: 64 tasks enqueued, 64 results returned, clean exit. ThreadSanitizer found a data race in the destructor on the same run. The writer was a free model; the reviewer was a sanitizer on a free server; the bug was a missing lock scope that no functional test could see.

Background

The project was a small batch hashing tool: walk a directory, compute SHA-256 for every file, write a manifest. The hashing is embarrassingly parallel, so the only interesting component was the worker pool. I generated the pool with a free model and verified it with the same pipeline I use for my own code: warnings on, functional tests, then sanitizers.

I used MonkeyCode's free model endpoint to generate the pool and its free server option to run the sanitizer builds. Disclosure: This article was prepared as part of MonkeyCode's product outreach. The workflow does not depend on either product; the commands below run on any Linux box with clang.

Goal

I defined "correct" before reading any generated code, in this order:

1. Every task enqueued before destruction either runs or is dropped safely — no crash, no use-after-free.
2. Destruction must not race with a worker that is still executing.
3. Enqueue after destruction throws instead of corrupting the queue.

Criterion 2 is the one that usually fails. It is also the one that functional tests rarely exercise.

Implementation

Step 1: The prompt

The prompt was deliberately short:

Write a minimal C++20 thread pool with these properties:
- fixed number of worker threads
- enqueue() returns std::future
- destructor stops workers and joins them
- safe to destroy while tasks are still queued
No dependencies beyond the standard library.

Step 2: The generated pool

The model returned a 60-line pool.h. It is the classic design: a mutex, a condition variable, a queue of std::function, and a stop_ flag.

// pool.h — generated by the free model, unmodified
#pragma once

#include <condition_variable>
#include <functional>
#include <future>
#include <mutex>
#include <queue>
#include <thread>
#include <vector>

class ThreadPool {
public:
    explicit ThreadPool(size_t n) : stop_(false) {
        for (size_t i = 0; i < n; ++i) {
            workers_.emplace_back([this] {
                for (;;) {
                    std::function<void()> task;
                    {
                        std::unique_lock<std::mutex> lock(mutex_);
                        cv_.wait(lock, [this] {
                            return stop_ || !tasks_.empty();   // line 22
                        });
                        if (stop_ && tasks_.empty()) return;
                        task = std::move(tasks_.front());
                        tasks_.pop();
                    }
                    task();
                }
            });
        }
    }

    template <class F>
    auto enqueue(F&& f) -> std::future<decltype(f())> {
        auto task = std::make_shared<std::packaged_task<decltype(f())()>>(
            std::forward<F>(f));
        auto res = task->get_future();
        {
            std::lock_guard<std::mutex> lock(mutex_);
            if (stop_) throw std::runtime_error("enqueue on stopped pool");
            tasks_.emplace([task] { (*task)(); });
        }
        cv_.notify_one();
        return res;
    }

    ~ThreadPool() {
        stop_ = true;            // line 49 — write WITHOUT the lock
        cv_.notify_all();
        for (auto& w : workers_) w.join();
    }

private:
    std::vector<std::thread> workers_;
    std::queue<std::function<void()>> tasks_;
    std::mutex mutex_;
    std::condition_variable cv_;
    bool stop_;
};

Nothing in this file looks wrong on first read. That is the point.

Step 3: The test harness

I wrote the test to destroy the pool while tasks are still in flight. A test that waits for every future before destruction never exercises the shutdown path.

// pool_test.cpp
#include "pool.h"
#include <atomic>
#include <chrono>
#include <cstdio>
#include <thread>

int main() {
    std::atomic<int> done{0};
    {
        ThreadPool pool(4);
        for (int i = 0; i < 64; ++i) {
            pool.enqueue([&done] {
                std::this_thread::sleep_for(std::chrono::microseconds(200));
                done.fetch_add(1, std::memory_order_relaxed);
            });
        }
    } // destroy while workers are still busy
    std::printf("completed %d of 64 tasks\n", done.load());
}

Results

Functional run: green

$ clang++ -std=c++20 -Wall -Wextra -O2 -pthread pool_test.cpp -o pool_test
$ ./pool_test
completed 64 of 64 tasks

No warnings, no crashes, all futures satisfied. A reasonable engineer would ship this. The model's output looked like the canonical implementation, because it was close to it — off by exactly one lock scope.

Sanitizer run: red

$ clang++ -std=c++20 -fsanitize=thread -O1 -g -pthread pool_test.cpp -o pool_test_tsan
$ ./pool_test_tsan
WARNING: ThreadSanitizer: data race (pid=3812)
  Write of size 1 at 0x7b1000004a80 by main thread:
    #0 ThreadPool::~ThreadPool() pool.h:49
    #1 main pool_test.cpp:14

  Previous read of size 1 at 0x7b1000004a80 by thread T2:
    #0 ThreadPool::ThreadPool()::$_0::operator()() const pool.h:22
    #1 std::thread::_State_impl...

I trimmed the libstdc++ frames; the two user frames are the whole story. Addresses and thread IDs change between runs. The shape of the report does not: an unsynchronized write in the destructor, an unsynchronized read in the worker predicate.

Why the functional test missed it

The write at line 49 is not protected by mutex_. A worker that is mid-task when the destructor runs will, on its next loop iteration, lock the mutex and evaluate the predicate at line 22. That read has no happens-before edge with the destructor's write. The window is a few microseconds, which is why 64/64 tasks completed and the process still exited with undefined behavior.

This is the failure mode that makes AI-generated concurrency code dangerous. It is not wrong in the way tests notice. It is wrong in the way the memory model notices.

The fix

One change: move the write under the lock.

     ~ThreadPool() {
-        stop_ = true;
+        {
+            std::lock_guard<std::mutex> lock(mutex_);
+            stop_ = true;
+        }
         cv_.notify_all();
         for (auto& w : workers_) w.join();
     }

Now the write is sequenced under the same mutex the workers hold when they read the flag. The happens-before edge is explicit, and the race is gone.

Re-run: clean

$ ./pool_test_tsan
completed 64 of 64 tasks

No warnings. Same binary, same test, one lock scope different.

Lessons learned

1. Functional tests verify behavior, not the memory model. The pool did the right thing every time and still contained undefined behavior.
2. Sanitizers are the cheapest second reviewer. The TSan build is a separate compile of the same 60-line file — cheap enough to run on every change.
3. Concurrency code from a free model gets the strongest review, not the weakest. The bug was in the one place humans also get wrong: shutdown.
4. If your test waits for all futures before destroying the pool, you are not testing destruction. Destroy while work is in flight.

Limitations

This approach is not universal. TSan has real overhead, so it belongs in a separate CI job, not as a replacement for the fast build. Some platforms need -fPIE -pie; on the free server image it worked out of the box. TSan does not catch every race — lock-free patterns need model checkers. And if nobody on the team can read a TSan report, the tool produces noise instead of signal.

Who should not use this workflow: teams with hour-long builds on shared runners, projects that already require model checkers, and anyone who treats a clean sanitizer run as proof of correctness rather than evidence.

The full source is in this article. The only difference between the racy and the fixed version is the destructor. If you want to reproduce the run, the commands above are complete — MonkeyCode's free server is one place to execute them, and any Linux box with clang behaves the same.