In C++ is there an idiomatic way to guard against the situation in which running a collection of actions causes the collection to be mutated?

c++ raii

189 观看

3回复

1339 作者的声誉

Say you have a class foo that wraps a collection of some kind of callable objects. foo has a member function run() that iterates over the collection and calls each function object. foo also has a member remove(...) that will remove a callable object from the collection.

Is there an idiomatic, RAII-style guard you can put in foo.run() and foo.remove(...) such that the removes that were driven by a call to foo.run() will be deferred until the guard's destructor fires? Can it be done with something in the standard library? Does this pattern have a name?

My current code seems inelegant so am looking for a best practice type solution.

Note: this is isnt about concurrency. A non thread-safe solution is fine. The issue is with re-entrancy and self-reference.

Here is an example of the problem, sans the inelegant "defer remove" guard.

class ActionPlayer
{
private:
    std::vector<std::pair<int, std::function<void()>>> actions_;
public:
    void addAction(int id, const std::function<void()>& action)
    {
        actions_.push_back({ id, action });
    }

    void removeAction(int id)
    {
        actions_.erase(
            std::remove_if(
                actions_.begin(),
                actions_.end(),
                [id](auto& p) { return p.first == id; }
            ),
            actions_.end()
        );
    }

    void run()
    {
        for (auto& item : actions_) {
            item.second();
        }
    }
};

then elsewhere:

...

ActionPlayer player;

player.addAction(1, []() {
    std::cout << "Hello there" << std::endl;
});

player.addAction(42, [&player]() {
    std::cout << "foobar" << std::endl;
    player.removeAction(1);
});

player.run(); // boom

Edit ... okay this is how I can see to do this via an RAII lock object. The following should handle actions throwing and re-entrant calls to run within run assuming the recursion eventually terminates (if not that is the user's fault). I used cached std::functions because in the real version of this code the equivalent of addAction and removeAction are template functions which cant just be stored in a vanilla homogeneously typed container.

class ActionPlayer
{
private:

    std::vector<std::pair<int, std::function<void()>>> actions_;
    int run_lock_count_;
    std::vector<std::function<void()>> deferred_ops_;

    class RunLock
    {
    private:
        ActionPlayer* parent_;
    public:
        RunLock(ActionPlayer* parent) : parent_(parent) { (parent_->run_lock_count_)++; }
        ~RunLock()
        {
            if (--parent_->run_lock_count_ == 0) {
                while (!parent_->deferred_ops_.empty()) {
                    auto do_deferred_op = parent_->deferred_ops_.back();
                    parent_->deferred_ops_.pop_back();
                    do_deferred_op();
                }
            }
        }
    };

    bool isFiring() const
    {
        return run_lock_count_ > 0;
    }

public:
    ActionPlayer() : run_lock_count_(0)
    {
    }

    void addAction(int id, const std::function<void()>& action)
    {
        if (!isFiring()) {
            actions_.push_back({ id, action });
        } else {
            deferred_ops_.push_back(
                [&]() {
                    addAction(id, action);
                }
            );
        }
    }

    void removeAction(int id)
    {
        if (!isFiring()) {
            actions_.erase(
                std::remove_if(
                    actions_.begin(),
                    actions_.end(),
                    [id](auto& p) { return p.first == id; }
                ),
                actions_.end()
            );
        } else {
            deferred_ops_.push_back(
                [&]() {
                    removeAction(id); 
                }
            );
        }
    }

    void run()
    {
        RunLock lock(this);
        for (auto& item : actions_) {
            item.second();
        }
    }
};
作者: jwezorek 的来源 发布者: 2017 年 12 月 27 日

回应 3


1

13058 作者的声誉

Usual way is to create a copy of the vector. But this may cause the removed actions to be run again.

void run()
{
    auto actions_copy{actions_};
    for (auto& item : actions_copy) {
        item.second();
    }
}

Other options if running removed action is not allowed

  1. Add a bool to store if some action is removed
  2. Use shared/weak ptr
  3. use std::list if it is known that the current action will not be removed.
作者: balki 发布者: 2017 年 12 月 27 日

1

22159 作者的声誉

Add a flag to run that says you're enumerating thru actions_. Then if removeAction is called with that flag set, you store the id in a vector for later removal. You might also need a separate vector to hold actions that are added while enumerating. Once you're all done iterating thru actions_, you remove the ones that want removed and add the ones that got added.

Something like

// within class ActionPlayer, add these private member variables
private:
    bool running = false;
    std::vector<int> idsToDelete;

public:
    void run() {
        running = true;
        for (auto& item : actions_) {
            item.second();
        }
        running = false;
        for (d: idsToDelete)
            removeAction(d);
        idsToDelete.clear();
    }

    // ...

You can make a similar change for deferred addAction calls (which you'll need to do if any of the actions can add an action, since the add might cause the vector to allocate more storage, invalidating all iterators to the vector).

作者: 1201ProgramAlarm 发布者: 2017 年 12 月 27 日

0

1172 作者的声誉

I would modify the structure slightly. Instead of allowing ActionPlayer to be modified directly, I would force all modifications through an external modifier class. In this example I made it an abstract Modifier class which can have different concrete implementations (e.g. DeferredModifier, InstantModifier, NullModifier, LoggedModifier, TestModifier .etc.). Your actions now only need to take a reference to the abstract base class of a modifier and call any add/remove .etc. function on that as necessary.. This allows separation of modification policy from the action implementation, and injection of different modification policies into actions.

This should also allow simpler support for concurrent modification, as you no longer need to toggle the run / not running state to defer modifications.

This example shows a simple way of replaying the actions in order (which is a property I assume you want to maintain). A more advanced implementation could scan the modification list backwards removing all add/remove pairs and then group modifications / removals to minimise copying when modifying the actions list.

Something like:

class ActionPlayer {
friend class Modifier;
...

    void run(Modifier &modifier) { }
private:
    void addAction(...) { ... }
    void removeAction(...) { ... }
}

class Modifier
{
public:
    virtual ~Modifier() {}
    virtual addAction(...) = 0;
    virtual removeAction(...) = 0;
}

class DelayedModifier : public Modifier
{
    struct Modification { virtual void run(ActionPlayer&) = 0; }

    struct RemoveAction : public Modification
    {
        int id;

        Removal(int _id) : id(_id) {} 
        virtual void run(ActionPlayer &player) { player.removeAction(id); }
    }

    struct AddAction : public Modification
    {
        int id;
        std::function<void(Modifier&)>> action;

        AddAction(int _id, const std::function<void(Modifier&)> &_action) : id(_id), action(_action)  {}
        virtual void run(ActionPlayer &player) { player.addAction(id, action) };
    }

    ActionPlayer &player;
    std::vector<Modification> modifications;

public:
    DelayedModifier(ActionPlayer &_player) player(_player) {}
    virtual ~DelayedModifier() { run(); }

    virtual void addAction(int id, const std::function<void(Modifier&)> &action) { modifications.push_back(AddAction(id, action)); }
    virtual void removeAction(int id) { modifications.push_back(RemoveAction(id)); }

    void run()
    {
        for (auto &modification : modifications)
            modification.run(player);
        modifications.clear();
    }
};

So you now write:

ActionPlayer player;

{
    DelayedModifier modifier(player);

    modifier.addAction(...);
    modifier.addAction(...);
    modifier.run();
    actions.run(modifier);
}
作者: Jarra McIntyre 发布者: 2017 年 12 月 27 日
32x32