c++ - 将自身从拥有它的容器中删除的shared_ptr，有更好的方法吗？

Question

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

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

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

另一个问题是，如果有人决定迭代 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

有一个很好的设计原则，即每个类都应该有一个目的。应该存在一个“任务对象”来执行该任务。当你开始增加额外的责任时，你往往会陷入困惑。困惑可能包括必须记住在完成主要目的后调用某个方法，或者必须记住使用一种 hacky 解决方法来保持对象处于事件状态。困惑往往是设计考虑不周的表现。对困惑感到不满意充分说明了您优秀设计的潜力。

让我们回溯一下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();
    }

}

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