c++ - 从拥有它的容器中删除自身的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

有一个很好的设计原则，说每个类都应该有一个正确的目的。应该存在一个“任务对象”来执行该任务。当您开始添加其他职责时，您往往会陷入困惑。会展可能包括必须记住在完成主要目的之后调用某个方法，或者必须记住使用hacky解决方法使对象保持 Activity 状态。展览通常是设计中思想不足的标志。对一团糟感到不满充分说明了您拥有良好设计的潜力。

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

因此，我们将重新设想您的类(class)，而不会造成“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();
    }

}

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