如果shared_ptr将自己从拥有它的容器中移除，有没有更好的方法？

Question

我想要做的基本上是将一堆任务对象排到一个容器中，在容器中任务可以从队列中删除自己。但我也不希望对象在移除自身时被销毁，这样它就可以继续完成正在做的任何工作。

因此，一种安全的方法是在工作完成时调用RemoveSelf()，或者使用keepAlive引用然后继续执行工作。我已经验证了这确实是有效的，而DoWorkUnsafe总是会在几次迭代后崩溃。

我对这个解决方案不是特别满意，因为我必须记得在工作结束时调用RemoveSelf()，或者记得使用keepAlive，否则会导致未定义的行为。

另一个问题是，如果有人决定遍历ownerList并执行工作，则在迭代器迭代时会使迭代器失效，这也是不安全的。

或者，我知道我可以将任务放到单独的“清理”队列中，然后分别销毁已完成的任务。但这种方法对我来说似乎更简洁，但有太多的警告。

有没有更好的模式来处理这样的事情？

#include <memory>
#include <unordered_set>

class SelfDestruct : public std::enable_shared_from_this<SelfDestruct> {
public:
    SelfDestruct(std::unordered_set<std::shared_ptr<SelfDestruct>> &ownerSet)
        : _ownerSet(ownerSet){}

    void DoWorkUnsafe() {
        RemoveSelf();
        DoWork();
    }

    void DoWorkSafe() {
        DoWork();
        RemoveSelf();
    }

    void DoWorkAlsoSafe() {
        auto keepAlive = RemoveSelf();
        DoWork();
    }


    std::shared_ptr<SelfDestruct> RemoveSelf() { 
        auto keepAlive = shared_from_this();
        _ownerSet.erase(keepAlive);
        return keepAlive;
    };

private:

    void DoWork() {
        for (auto i = 0; i < 100; ++i)
            _dummy.push_back(i);
    }

    std::unordered_set<std::shared_ptr<SelfDestruct>> &_ownerSet;
    std::vector<int> _dummy;
};

TEST_CASE("Self destruct should not cause undefined behavior") {

    std::unordered_set<std::shared_ptr<SelfDestruct>> ownerSet;

    for (auto i = 0; i < 100; ++i)
        ownerSet.emplace(std::make_shared<SelfDestruct>(ownerSet));

    while (!ownerSet.empty()) {
        (*ownerSet.begin())->DoWorkSafe();
    }
}

Answer 1

有一个很好的设计原则，即每个类应该只有一个目的。应该存在一个“任务对象”来执行该任务。当你开始添加额外的职责时，你往往会以一团糟告终。混乱可能包括在完成主要目的后必须记住调用某个方法，或者必须记住使用一种老套的变通办法来保持对象存活。乱七八糟的东西往往是设计思想不足的表现。对混乱的不满意很好地说明了你有潜力进行好的设计。

让我们回过头来看看the real problem。有一些任务对象存储在容器中。容器决定何时调用每个任务。在调用下一个任务之前，必须从容器中删除该任务(这样它就不会再次被调用)。在我看来，从容器中删除元素的责任应该落在容器身上。

因此，我们将重新设想您的类没有"SelfDestruct“混乱。任务对象的存在是为了执行任务。它们可能是多态的，因此需要一个指向任务对象的指针容器，而不是任务对象的容器。任务对象并不关心它们是如何管理的；这是为其他人工作。

class Task {
public:
    Task() {}
    // Other constructors, the destructor, assignment operators, etc. go here

    void DoWork() {
        // Stuff is done here.
        // The work might involve adding tasks to the queue.
    }
};

现在将注意力集中在容器上。容器(更准确地说，是容器的所有者)负责添加和删除元素。那就这么做吧。您似乎更喜欢在调用元素之前删除它。对我来说，这似乎是个好主意，但不要试图将任务中的删除操作作废。取而代之的是使用助手函数，将此逻辑保持在容器所有者的抽象层。

// Extract the first element of `ownerSet`. That is, remove it and return it.
// ASSUMES: `ownerSet` is not empty
std::shared_ptr<Task> extract(std::unordered_set<std::shared_ptr<Task>>& ownerSet)
{
    auto begin = ownerSet.begin();
    std::shared_ptr<Task> first{*begin};
    ownerSet.erase(begin);
    return first;
}

TEST_CASE("Removal from the container should not cause undefined behavior") {

    std::unordered_set<std::shared_ptr<Task>> ownerSet;

    for (int i = 0; i < 100; ++i)
        ownerSet.emplace(std::make_shared<Task>());

    while (!ownerSet.empty()) {
        // The temporary returned by extract() will live until the semicolon,
        // so it will (barely) outlive the call to DoWork().
        extract(ownerSet)->DoWork();
        // This is equivalent to:
        //auto todo{extract(ownerSet)};
        //todo->DoWork();
    }

}

从某种角度来看，这与您的方法相比几乎是微不足道的更改，因为我所做的只是将责任从任务对象转移到容器的所有者。然而，随着这种转变，混乱就消失了。执行相同的步骤，但它们是有意义的，并且在移动到更合适的上下文时几乎是强制的。整洁的设计往往导致整洁的实现。