这个线程池使用安全吗？

Question

我将几个作业发布到线程池，然后等待它完成。我想知道我是否在这里遗漏了什么，因为有时我的工作线程似乎冻结了。

我的主线程这样启动工人：

numJobsPosted = 0;
for(auto entry : list)
{
    numJobsPosted++;
    threadPool->post(std::bind(&Controller::workerFunc, this, entry));
}

std::unique_lock<std::mutex> lock(m_workerLock);
while(numJobsPosted > 0)
{
    m_workerCondition.wait(lock);
}

现在我的 workerFunc 看起来像这样：

void Controller::workerFunc(Entry entry)
{
    // do some work with entry

    // notify finished
    numJobsPosted--;
    if(numJobsPosted <= 0)
    {
        // does the look need to be around the numJobsPosted-- ?
        std::unique_lock<std::mutex> locker(m_workerLock);
        m_workerCondition.notify_one();
    }
}

上面的代码安全吗，还是我需要在减量运算符周围加锁？

Answer 1

这可能取决于线程池内部逻辑或设置的详细信息（例如，如果您有一个线程，那么作业实际上是按顺序运行的），但假设 numJobsPosted 是 int 或类似的内置类型，您的代码不是线程安全的。
这行在workerFunc:

numJobsPosted--;

如果它被多个作业同时执行，很可能会成为竞争条件的主题。

另外，我不确定你的线程池的 post 函数究竟做了什么，但如果它立即将工作函数分派到一个线程并且一些工作函数可以立即返回，那么你在这之间还有另一个可能的竞争条件主线程代码中的行：

numJobsPosted++;

还有workerFunc中的这一行：

numJobsPosted--;

为了安全起见，您可以将numJobsPosted 设为原子，例如像这样声明它（在 C++11 中）：

#include <atomic>
std::atomic_int numJobsPosted;

让你的workerFunc 变成这样：

void Controller::workerFunc(Entry entry)
{
    // do some work with entry

    // notify finished
    {
        std::unique_lock<std::mutex> locker(m_workerLock);
        numJobsPosted--;
        if(numJobsPosted <= 0)
        {
            m_workerCondition.notify_one();
        }
    }
}

可以解决第一个竞争条件情况，但不能解决第二个。

（另外，我不太了解您在 numJobsPosted 上进行的操作和测试的逻辑，但我认为这与您的问题无关）