Boost::signals2 passing invalid data

Question

I've been a C/C++ developer for about 20 years now, but templates have always been a weak spot for me. With template programming becoming ever more useful, and complicated, in the C++11 and C++14 standards, I decided to try an exercise to learn. I've been moderately successful, but I have an issue I'm having problems with. I have the following class:

namespace Events {
// Place your new EventManager events here
static const uint32_t StatsData = 0;
static const uint32_t StatsRequest = 1;
static const uint32_t StatsReply = 2;
static const uint32_t ApplianceStatsRequest = 3;
static const uint32_t ApplianceStatsReply = 4;
static const uint32_t NullEvent = 5;
};

class EventManager {
    public:
    static EventManager *instance() {
        if (Instance)
            return Instance;

        return new EventManager();
    };

    static void destroy() {
        delete Instance;
        Instance = nullptr;
    }

    template<typename T>
    bool consume_event(uint32_t event, std::function<T> func) {
        if (_event_map.find(event) == _event_map.end())
            // Create the signal, in true RAII style
            _event_map[event] = new boost::signals2::signal<T>();

        boost::any_cast<boost::signals2::signal<T> *>(_event_map[event])->connect(func);

        return true;
    }

    void emit(uint32_t event) {
        if (_event_map.find(event) == _event_map.end())
            return;

        try {
            boost::signals2::signal<void()> *sig =
                boost::any_cast<boost::signals2::signal<void()> *>(_event_map[event]);

                (*sig)();
        }
        catch (boost::bad_any_cast &e) {
            SYSLOG(ERROR) << "Caught instance of boost::bad_any_cast: " << e.what();
            abort();
        }
    }

    template<typename... Args>
    void emit(uint32_t event, Args... args) {
        if (_event_map.find(event) == _event_map.end())
            return;

        try {
            boost::signals2::signal<void(Args...)> *sig =
                boost::any_cast<boost::signals2::signal<void(Args...)> *>(_event_map[event]);
            (*sig)(args...);
        }
        catch (boost::bad_any_cast &e) {
            SYSLOG(ERROR) << "Caught instance of boost::bad_any_cast: " << e.what();
            abort();
        }
    }

private:
    EventManager() { Instance = this; }; 
    ~EventManager() { Instance = nullptr; };

    static EventManager *Instance;
    std::map<uint32_t, boost::any> _event_map;
};

This code would potentially go into a large framework that loads multiple modules which are dynamic libraries on linux. The idea would be for a given module to be able to call:

consume_event<ParamTypes><EventNumber, SomeCallack)

The callback may be a function with signature void(ParamTypes), or the result of std::bind() on a function with signature void(ParamTypes).

Another module would then be able to call:

emit<ParamTypes>(EventNumber, ParamValues) 

and each module that had called consume_event, would have it's handler called with ParamValues.

This seems to work in almost every case, except when I pass a reference to a custom class, like this:

std::cout << "Sending stats data with ref: " << std::hex << ip_entry.second <<  std::endl;
emit<ip_stats_t &>(Events::StatsData, *ip_entry.second);

In this case, the function that is connected to the signal, receives 0xa, and promptly crashes when it tries to treat it as an ip_stats_t &.

The output is:

Sending stats data with ref: 0x7fbbc4177d50 <- This is the output of the line seen above
ips addr: 0xa << this is from the function that gets called by the signal.

Update: I just noticed it does the same thing when passing any variable by reference, not just the custom class above.

Additionally, please note that there is no SSCCE in this question because any SSCCE invariable works. The problem does not occur until the working code is put into the above framework.

Update2: The real question here is, how can this design be made better. This one not only doesn't work properly, but syntactically, it stinks. it's ugly, inelegant, and really, there's nothing good about it, except that it did what I wanted it to do and increased my understanding of templates.

Update3: I have now 100% confirmed that this has nothing to do with the data type that I'm passing. If I pass any variable by reference, the slot always receives 0xa as the address of the reference. This includes std::strings, and even ints. If I pass any variable by value, the copy constructor of that value eventually receives 0xa as the reference of the value to copy from. This only happens when calling a slot in module B from a signal created in module A. What am I missing?

Any ideas? Thanks!

Answer 1

The following code, which is Sehe's revised code without the boost::signals solved my problem completely. It would appear that boost::signals was having issues passing any data whatsoever across module boundries. Replacing it with a simple vector of functions works in all cases, and is faster anyway!

enum class EventId : uint32_t {
    // Place your new EventManager events here
    StatsData             = 0,
    StatsRequest          = 1,
    StatsReply            = 2,
    ApplianceStatsRequest = 3,
    ApplianceStatsReply   = 4,
};

struct ip_stats_t;

namespace Events {
    template <EventId> struct Traits;

    template <> struct Traits<EventId::StatsData>             
        { using signal_vec = std::vector<std::function<void(ip_stats_t &)>>; } ;

    template <> struct Traits<EventId::StatsRequest>          
        { using signal_vec = std::vector<std::function<void(std::ostream &)>>; } ;

    template <> struct Traits<EventId::StatsReply>            
        { using signal_vec = std::vector<std::function<void(std::string &)>>; } ;

    template <> struct Traits<EventId::ApplianceStatsRequest> :
        Traits<EventId::StatsRequest> {};

    template <> struct Traits<EventId::ApplianceStatsReply> : 
        Traits<EventId::StatsReply> {}; 
}

class EventManager {
    public:
        static EventManager& instance() {
            static EventManager instance;
            return instance;
        };

        template <EventId event, typename F>
        void consume_event(F&& func) {
            get_slot<event>().push_back(std::forward<F>(func));
        }

        template <EventId event, typename... Args>
        void emit(Args&&... args) {
            for (auto &vi : get_slot<event>()) {
                vi(std::forward<Args>(args)...);
            }
        }

    private:
        template <EventId event, 
            typename Slot = typename Events::Traits<event>::signal_vec,
            typename SlotPtr = std::shared_ptr<Slot>>
        Slot& get_slot() {
            if (_event_map.find(event) == _event_map.end())
                _event_map.emplace(event, std::make_shared<Slot>());

            try {
                return *boost::any_cast<SlotPtr>(_event_map[event]);
            }
            catch (boost::bad_any_cast const &e) {
                std::cerr << e.what() << " on event #" << static_cast<uint32_t>(event) << "\n";
                abort();
            }
        }

    EventManager() = default;
    std::map<EventId, boost::any> _event_map;
};

Answer 2

UPDATED I've since come up with a demonstration that would appear to be closer to what you were trying to achieve:

@lk75 For fun, here's an approach that abstracts the event mechanism in a fairly extensible way, while

• not being overly complicated
• not requiring calling signature to be repeated all over the place (it's in Traits now)
• not leaking signals by using true RAII style (SCNR). No more use of new or delete!

See it Live On Coliru.

Note how I simplified the singleton and turned both consume_event and emit into one-liners now:

    static EventManager& instance() {
        static EventManager instance;
        return instance;
    };

    template <EventId event, typename F>
    bool consume_event(F&& func) {
        get_slot<event>().connect(std::forward<F>(func));
        return true;
    }

    template <EventId event, typename... Args>
    void emit(Args&&... args) {
        get_slot<event>()(std::forward<Args>(args)...);
    }

Full Code

For reference:

Live On Coliru

#include <boost/any.hpp>
#include <boost/make_shared.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/signals2/signal.hpp>
#include <iostream>
#include <memory>
#include <string>

struct ip_stats_t {
    std::string canary;
};

enum class EventId : uint32_t {
    // Place your new EventManager events here
    StatsData             = 0,
    StatsRequest          = 1,
    StatsReply            = 2,
    ApplianceStatsRequest = 3,
    ApplianceStatsReply   = 4,
    NullEvent             = 5, // Not implemented
};

namespace Events {

    template <EventId> struct Traits;

    template <> struct Traits<EventId::StatsData>             { using signal_type = boost::signals2::signal<void(int)>;                 } ;
    template <> struct Traits<EventId::StatsRequest>          { using signal_type = boost::signals2::signal<void(bool, bool)>;          } ;
    template <> struct Traits<EventId::StatsReply>            { using signal_type = boost::signals2::signal<void(std::string)>;         } ;
    template <> struct Traits<EventId::ApplianceStatsRequest> { using signal_type = boost::signals2::signal<void(double, ip_stats_t&)>; } ;
  //template <> struct Traits<EventId::NullEvent>             { using signal_type = boost::signals2::signal<void()>;                    } ;

    template <> struct Traits<EventId::ApplianceStatsReply> : Traits<EventId::ApplianceStatsRequest> { }; 
}

class EventManager {
  public:
    static EventManager& instance() {
        static EventManager instance;
        return instance;
    };

    template <EventId event, typename F>
    bool consume_event(F&& func) {
        get_slot<event>().connect(std::forward<F>(func));
        return true;
    }

    template <EventId event, typename... Args>
    void emit(Args&&... args) {
        get_slot<event>()(std::forward<Args>(args)...);
    }

  private:
    template <EventId event, typename Slot = typename Events::Traits<event>::signal_type, typename SlotPtr = boost::shared_ptr<Slot> >
    Slot& get_slot() {
        try {
            if (_event_map.find(event) == _event_map.end())
                _event_map.emplace(event, boost::make_shared<Slot>());

            return *boost::any_cast<SlotPtr>(_event_map[event]);
        }
        catch (boost::bad_any_cast const &e) {
            std::cerr << "Caught instance of boost::bad_any_cast: " << e.what() << " on event #" << static_cast<uint32_t>(event) << "\n";
            abort();
        }
    }

    EventManager() = default;
    std::map<EventId, boost::any> _event_map;
};

int main() {
    auto& emgr = EventManager::instance();

    emgr.consume_event<EventId::ApplianceStatsRequest>([](double d, ip_stats_t& v) { 
            std::cout << "d: " << d << ", v.canary: " << v.canary << "\n";
        });
    emgr.consume_event<EventId::ApplianceStatsRequest>([](double d, ip_stats_t& v) { 
            std::cout << "And you can register more than one\n";
        });


    ip_stats_t v { "This is statically checked" };
    emgr.emit<EventId::ApplianceStatsRequest>(3.142f, v);

    emgr.emit<EventId::StatsData>(42); // no connection, but works
    emgr.consume_event<EventId::StatsData>([](int) { std::cout << "Now it's connected\n"; });
    emgr.emit<EventId::StatsData>(42); // now with connection!

#if 0
    emgr.emit<EventId::ApplianceStatsRequest>();  // error: no match for call to ‘(boost::signals2::signal<void(double, ip_stats_t&)>) ()’
    emgr.consume_event<EventId::NullEvent>([]{}); // use of incomplete type Traits<NullEvent>
#endif
}

---

Old answer:

You seem to have trouble with the variadic forwarding:

    (*sig)(std::forward<Args>(args)...);

Also, forwarding really makes sense only when taking the arguments by "universal reference":

template<typename... Args>
void emit(uint32_t event, Args&&... args) { // NOTE!!

However, you do not rely on argument type deduction to get the actual value categories (rvalue vs. lvalue). And, rightly so (because the compiler would likely never get the exact argument types "right" to match the stored signal (making the any_cast fail at best, or invoke Undefined Behaviour at best.)

So in this case, you should dispense with the whole forwarding business:

template<typename... Args> using Sig = boost::signals2::signal<void(Args...)>;

template<typename... Args>
void emit(uint32_t event, Args... args) {
    if (_event_map.find(event) == _event_map.end())
        return;

    try {
        Sig<Args...> *sig = boost::any_cast<Sig<Args...> *>(_event_map[event]);

        (*sig)(args...);
    }
    catch (boost::bad_any_cast &e) {
        std::cerr << "Caught instance of boost::bad_any_cast: " << e.what();
        abort();
    }
}

Full demo program: Live On Coliru

#include <boost/any.hpp>
#include <boost/signals2/signal.hpp>
#include <iostream>
#include <string>

struct ip_stats_t { 
    std::string canary;
};

template<typename... Args> using Sig = boost::signals2::signal<void(Args...)>;
std::map<uint32_t, boost::any> _event_map;

template<typename... Args>
void emit(uint32_t event, Args&&... args) {
    if (_event_map.find(event) == _event_map.end())
        return;

    try {
        Sig<Args...> *sig = boost::any_cast<Sig<Args...> *>(_event_map[event]);

        (*sig)(std::forward<Args>(args)...);
    }
    catch (boost::bad_any_cast &e) {
        std::cerr << "Caught instance of boost::bad_any_cast: " << e.what();
        abort();
    }
}

int main()
{
    Sig<int, double> sig1;
    Sig<ip_stats_t&> sig2;

    sig1.connect([](int i, double d) { std::cout << "Sig1 handler: i = " << i << ", d = " << d << "\n"; });
    sig2.connect([](ip_stats_t& v)   { std::cout << "Sig2 handler: canary = " << v.canary << "\n"; });

    _event_map[1] = &sig1;
    _event_map[2] = &sig2;

    emit<int, double>(1, 42, 3.14);

    ip_stats_t instance { "Hello world" }, *ptr = &instance;

    emit<ip_stats_t&>(2, *ptr);
}