Tag: callback

With this post, i would like to 1) ask for feedback on below code as it stands:

• do i apply all best practices for c++20?
• is it safe?
• is my way to update a callback from inside a running callback sensible/best?

Based on code and design advice found on stackoverflow (e.g. here), I have written a class that handles listeners registering callbacks to receive messages in multithreaded code (listeners can be added from different threads than the thread that messages are sent from, only one thread every sends messages). This code also needed multiple ways for listeners to unregister themselves:

1. when the class owning the receiver destructs or is otherwise done receiving: hence add_listener() returns a cookie in the form of a std::list::const_iterator (list because iterators to none of the other elements are invalidated when an element is removed, and other cookies thus stay valid)
2. when the callback during execution determines it no longer needs further messages, this is handled by returning true (“remove myself please”).

I now run into the situation where a listener callback, when invoked, needs to replace itself with another callback. I can’t call add_listener() as that would deadlock when called from inside an invoked callback. Unless i move to use a std::recursive_mutex instead. That seems heavy.

So instead i have added a replacement list to the broadcaster storing a pair of <listenerCookie, listener> which lists what updates should be done once the current message has been broadcasted to all listeners. The updates are performed at the end of the notify_all function. This guarantees that the old callback is never called again and that the cookie held stays valid but now points to the new callback. But this design seems complicated and unsafe: it required adding an extra member function to the broadcaster class, which should only be called from inside a callback. And that is easy to misuse unless code comments (and the horrible function name :p) saying to only invoke from inside a callback are honored. Or is there a way to ensure a function can only be called by the invoked callback?

Am i missing some simpler solution?

A simpler, but sadly impossible solution i considered was for the callback to return the new callback to register, which would replace the currently registered one (the cookie thus stays valid). But I do not know what my listener’s function signature would then be, as you cannot do something CRTP-like with using statements (using listener = std::function<listener(Message...)>; is invalid) (NB: in reality if this option were possible i’d return a variant with bool as well i guess, as i need to also maintain self-deregistration functionality.)

#include <list>
#include <mutex>
#include <functional>
#include <vector>
#include <utility>

template <class... Message>
class Broadcaster
{
public:
    using listener       = std::function<bool(Message...)>;
    using listenerCookie = typename std::list<listener>::const_iterator;

    listenerCookie add_listener(listener&& r_)
    {
        auto l = lock();
        return targets.emplace(targets.cend(), std::move(r_));
    }
    void remove_listener(listenerCookie it_)
    {
        auto l = lock();
        remove_listener_impl(it_);
    }

    void notify_all(Message... msg_)
    {
        auto l = lock();

        for (auto it = targets.cbegin(), e = targets.cend(); it != e; )
            if ((*it)(msg_...))
                it = remove_listener_impl(it);
            else
                ++it;

        if (!update_list.empty())
        {
            for (auto&& (c_it, r) : update_list)
            {
                // turn cookie into non-const iterator
                auto it = targets.erase(c_it, c_it);    // NB, (c_it, c_it) is an empty range, so nothing is removed
                // update callback
                *it = std::move(r);
            }
            update_list.clear();
        }
    }

    // NB: should only be called from running callback!
    void update_callback_from_inside_callback(listenerCookie it_, listener&& r_)
    {
        update_list.emplace_back(it_, std::move(r_));
    }

private:
    auto lock()
    {
        return std::unique_lock<std::mutex>(m);
    }
    listenerCookie remove_listener_impl(listenerCookie it_)
    {
        return targets.erase(it_);
    }

private:
    std::mutex m;
    std::list<listener> targets;

    std::vector<std::pair<listenerCookie, listener>> update_list;
};