Garmaine Staff asked 2 years ago

What I want to do is basically queue a bunch to task objects to a container, where the task can remove itself from the queue. But I also don't want the object to be destroyed when it removes itself, so it can continue to finish whatever the work is doing.

So, a safe way to do this is either call RemoveSelf() when the work is done, or use a keepAlive reference then continue to do the work. I've verified that this does indeed work, while the DoWorkUnsafe will always crash after a few iterations.

I'm not particularly happy with the solution, because I have to either remember to call RemoveSelf() at the end of work being done, or remember to use a keepAlive, otherwise it will cause undefined behavior.

Another problem is that if someone decides to iterate through the ownerList and do work, it would invalidate the iterator as they iterate, which is also unsafe.

Alternatively, I know I can instead put the task onto a separate "cleanup" queue and destroy finished tasks separately. But this method seemed neater to me, but with too many caveats.

Is there a better pattern to handle something like this?

#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();
    }
}