自擦除工人c++

Self erasing workers c++

本文关键字：c++ 擦除      更新时间：2023-10-16

谁能解释一下为什么这个c++代码崩溃了(实际上它没有崩溃，而是valgrind抱怨无效的内存访问)

这个想法很简单。控制器创建一些工作者。每个worker都有一个functor对象，一旦它完成了它的工作，它就会擦除这个worker。谁能提出解决办法?

typedef boost::function<void()> Callback;
struct Worker
{
    void start(Callback onFinnish)
    {
        // Called when finnished working
        onFinnish();
    }
};
typedef boost::shared_ptr<Worker> WorkerPtr;
struct Controler
{
    void start()
    {
        for(int i = 0; i < 5; ++i)
        {
            auto workerPtr = boost::make_shared<Worker>();
            workers.insert(workerPtr);
        }
        for(const auto &workerPtr: workers)
        {
            workerPtr->start(
                [this, workerPtr] ()
                {
                    workers.erase(workerPtr);
                    if(workers.size() == 0)
                    {
                        std::cout << "All workers done!" << std::endl;
                    }
                }
            );
        }
    }
    std::set<WorkerPtr> workers;
};
// Somewhere in code
Controler c; c.start();

EDIT AFTER COMMENTS:我像这样实现了for自动循环，现在它工作了:

    for(auto workerIt = workers.begin(); workerIt != workers.end();)
    {
        auto nextWorker = workerIt; ++nextWorker;
        (*workerIt)->start(
            [this, workerIt] ()
            {
                workers.erase(workerIt);
                if(workers.empty())
                {
                    onWorkersDone();
                }
            }
        );
        workerIt = nextWorker;
    }

在for范围内，修改循环内的容器，这涉及到UB。

创建一个集合的副本应该可以解决您的问题:

for(const auto &workerPtr: std::set<WorkerPtr>(workers)) // create a copy of the set
{
    workerPtr->start(
        [this, workerPtr] ()
        {
            workers.erase(workerPtr);
            if(workers.size() == 0)
            {
                std::cout << "All workers done!" << std::endl;
            }
        }
    );
}
编辑:

正如Dietmar khl所指出的，如果方法start启动了一个新线程，那么您将在set上产生数据竞争。

根据6.5.4 [stmt.]基于范围的for循环的行为类似于这段代码:

{
    auto && __range = workers;
    for (auto __begin = workers.begin(), __end = workers.end(); __begin != __end; ++__begin ) {
        const auto& = *__begin;
        <body-of-the loop>
    }
 }

因为你很高兴地清除了当前被引用的对象，实际上你在循环的每次迭代中使__begin迭代器无效:对于关联容器，所有迭代器和指向被擦除对象的指针/引用都无效。任何失效的迭代器都是一个等待解除武装的陷阱(通过赋值或销毁)，当以其他方式使用它时，它将在未定义的行为中爆炸。

就我个人而言，我会通过迭代集合并以更有效的方式删除其元素来解决这个问题:你的方法需要在每次迭代中搜索元素，这是不必要的成本。你可以只保留一个迭代器:

for (auto it = works.begin(), end = works.end(); it != end; ) {
    (*it)->start([this, it]() {
         workers.erase(it++);
         if (workers.empty()) {
             std::cout << "All workers done!n";
         }
     });
}

当然，您还应该使用container.empty()来确定容器是否为空. ...不要使用std::endl，因为它对任何人都没有任何好处。使用std::flush，如果你真的想刷新流。

当然，如果start()在发布的代码实际上是一个简化，实际上启动一个线程，问题就变成了一个不同的!在这种情况下，当创建函数对象以启动线程时，不会执行erase()语句。最初的问题，即基于范围的for的迭代器无效，只会发生在启动线程执行并且erase()在启动线程继续前进之前删除对象的情况下!

相反，有一个明显的数据竞争:有多个线程erase()从this->workers对象没有任何同步!您可以通过使用合适的互斥锁保护对共享std::set<WorkerPtr>的访问来解决这个问题。例如，我认为这段代码解决了这个问题:

struct Controler
{
    void start()
    {
        for(int i = 0; i < 5; ++i)
        {
            auto workerPtr = boost::make_shared<Worker>();
            this->workers.insert(workerPtr);
        }
        std::lock_guard<std::mutex> kerberos(this->mutex);
        for(const auto &workerPtr: this->workers)
        {
            workerPtr->start(
                [this, workerPtr] ()
                {
                    std::lock_guard<std::mutex> kerberos(this->mutex);
                    this->workers.erase(workerPtr);
                    if(this->workers.empty()) {
                        std::cout << "All workers done!n";
                    }
                }
            );
        }
    }
    std::set<WorkerPtr> workers;
    std::mutex          mutex;
};

当然，问题分析和解决假设workerPtr->start(...)实际上启动了一个线程来完成这项工作!如果工作在同一个线程上执行，来自基于范围的for的迭代器实际上是无效的，并且像上面的代码中那样添加互斥锁会导致死锁。

相关文章：