这个线程池使用安全吗？

Question

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

我的主线程是这样启动worker的：

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 函数究竟做了什么，但如果它立即将工作函数分派给线程并且一些工作函数可以立即 return，您在主线程代码中的这一行之间还有另一个可能的竞争条件：

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 上进行的操作和测试的逻辑，但我认为这与您的问题无关）

这个线程池使用安全吗？

Is this threadpool usage safe?

c++

multithreading

locking

thread-safety

threadpool