P i
P i

Reputation: 30684

Robust C++ event pattern

I've put together a simple C++ event pattern, that allows the following:

struct Emitter {
    Event<float> ev;    
    void triggerEvent() { ev.fire(42.0); }
};

struct Listener {
    void gotEvent(float x) { ... }
};

int main() {
    // event source and listener unaware of each other's existence
    Emitter emitter(); 
    Listener listener();

    // hook them up
    emitterA.ev.addSubscriber(&listener, &Listener::gotEvent);
    {
        Listener listener2();
        emitter.ev.addSubscriber(&listener2, &Listener::gotEvent);
        emitter.triggerEvent();
        emitter.ev.removeSubscriber(&listener2); 
        // ^ PROBLEM!
    }

    emitter.triggerEvent();

    emitter.ev.removeSubscriber(&listener1); 
}

The problem is that the developer is required to manually remove each subscriber, otherwise the event's fire(), in iterating through all subscribers acting on each, will end up triggering a method on an object that may or may not still be in existence.

Here is the complete code, together with a working example: http://coliru.stacked-crooked.com/a/8bb20dacf50bf073

I will paste below for posterity.

If I comment out the offending line 99, it still works! But this is clearly only because the memory hasn't yet been overwritten.

That's a dangerous bug, because it could lay dormant.

How can I code in such a way that doesn't expose me to this potential UB bug?

Is there any way my line 35 ..

template<class... Args> 
class Event {
    :
    void fire(Args... args) {
        for( auto& f : subscribers )
            f->call(args...);

could somehow detect if each subscriber is still in existence...

While still preserving the fact that emitter and subscriber don't know of each other's existence?

Complete listing:

#include <vector>
#include <iostream>
#include <algorithm>
#include <memory>

using namespace std;

template<class... Args> 
class SubscriberBase {
public:
    virtual void call(Args... args) = 0;
    virtual bool instanceIs(void* t) = 0;
    virtual ~SubscriberBase() { };
};

template<class T, class... Args> 
class Subscriber : public SubscriberBase<Args...> {
private:
    T* t;
    void(T::*f)(Args...);
public:
    Subscriber(T* _t, void(T::*_f)(Args...)) : t(_t), f(_f)  { }
    void call(Args... args)   final  { (t->*f)(args...); }
    bool instanceIs(void* _t) final  { return _t == (void*)t; }
    ~Subscriber()             final  { cout << "~Subscriber() hit! \n"; }
};

template<class... Args> 
class Event {
private:
    using SmartBasePointer = unique_ptr<SubscriberBase<Args...>>;
    std::vector<SmartBasePointer> subscribers;
public:
    void fire(Args... args) {
        for( auto& f : subscribers )
            f->call(args...);
    }

    template<class T> 
    void addSubscriber( T* t, void(T::*f)(Args... args) ) {
        auto s = new Subscriber <T, Args...>(t, f);
        subscribers.push_back(SmartBasePointer(s));
    }

    template<class T> 
    void removeSubscriber(T* t) {
        auto to_remove = std::remove_if(
            subscribers.begin(), 
            subscribers.end(),  
            [t](auto& s) { return s->instanceIs((void*)t); }
            );
        subscribers.erase(to_remove, subscribers.end()); 
    }
};

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

// example usage:
class Emitter {
private:
    string name;
public:    
    Event<float> eventFloat;
    Event<bool, int> eventB;

    Emitter(string _name) : name(_name) { }

    void triggerEvent() { 
        cout << name << "::triggerEvent() ~ Firing event with: 42\n";
        eventFloat.fire(42.0f); 
    }
};

struct Listener {
    string name;
    Listener(string _name) 
        : name(_name)  { 
        cout << name << "()\n";
    }
    ~Listener() { 
        cout << "~" << name << "()\n";
        //emitter.eventFloat.removeSubscriber(this); 
    }

    void gotEvent(float x) { cout << name <<"::gotEvent hit with value: " << x << endl; }
};

int main() {
    // event source and listener unaware of each other's existence
    Emitter emitterA("emitterA"); 
    Listener listener1("listener1");

    // hook them up
    emitterA.eventFloat.addSubscriber(&listener1, &Listener::gotEvent);
    {
        Listener listener2("listener2");
        emitterA.eventFloat.addSubscriber(&listener2, &Listener::gotEvent);
        emitterA.triggerEvent();
        //emitterA.eventFloat.removeSubscriber(&listener2); // hmm this is awkward
    }

    emitterA.triggerEvent();

    emitterA.eventFloat.removeSubscriber(&listener1); 
    emitterA.triggerEvent();

    return 0;
}

Upvotes: 2

Views: 5068

Answers (2)

P i
P i

Reputation: 30684

I have described my solution here: http://www.juce.com/forum/topic/signals-slots-juce#comment-321103

http://coliru.stacked-crooked.com/a/b2733e334f4a5289

#include <vector>
#include <iostream>
#include <algorithm>
#include <memory>
#include <string>

using namespace std;

// an event holds a vector of subscribers
// when it fires, each is called

template<class... Args>
class SubscriberBase {
public:
    virtual void call(Args... args) = 0;
    virtual bool instanceIs(void* t) = 0;
    virtual ~SubscriberBase() { };
};

template<class T, class... Args>
class Subscriber : public SubscriberBase<Args...> {
private:
    T* t;
    void(T::*f)(Args...);
public:
    Subscriber(T* _t, void(T::*_f)(Args...)) : t(_t), f(_f) { }
    void call(Args... args)   final { (t->*f)(args...); }
    bool instanceIs(void* _t) final { return _t == (void*)t; }
    ~Subscriber()             final { cout << "~Subscriber() hit! \n"; }
};

// our Listener will derive from EventListener<Listener>
// which holds a list of a events it is subscribed to.
// As these events will have different sigs, we need a base-class.
// We will store pointers to this base-class.
class EventBase { 
public:
    virtual void removeSubscriber(void* t) = 0;
};

template<class... Args>
class Event : public EventBase {
private:
    using SmartBasePointer = unique_ptr<SubscriberBase<Args...>>;
    std::vector<SmartBasePointer> subscribers;
public:
    void fire(Args... args) {
        for (auto& f : subscribers)
            f->call(args...);
    }

    template<class T>
    void addSubscriber(T* t, void(T::*f)(Args... args)) {
        auto s = new Subscriber <T, Args...>(t, f);
        subscribers.push_back(SmartBasePointer(s));
    }

    //template<class T>
    void removeSubscriber(void* t) final {
        auto to_remove = std::remove_if(
            subscribers.begin(),
            subscribers.end(),
            [t](auto& s) { return s->instanceIs(t); }
        );
        subscribers.erase(to_remove, subscribers.end());
    }
};

// derive your listener classes: struct MyListener : EventListener<MyListener>, i.e. CRTP
template<class Derived>
class EventListener {
private:
    // all events holding a subscription to us...
    std::vector<EventBase*> events;

public:
    template<class... Args>
    void connect(Event<Args...>& ev, void(Derived::*listenerMethod)(Args... args)) {
        ev.addSubscriber((Derived*)this, listenerMethod);
        events.push_back(&ev);
    }

    // ...when the listener dies, we must notify them all to remove subscription
    ~EventListener() {
        for (auto& e : events)
            e->removeSubscriber((void*)this);
    }
};

// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

// example usage:
class Publisher {
private:
    string name;
public:
    Event<float> eventFloat;
    Event<bool, int> eventB;

    Publisher(string _name) : name(_name) { }

    void triggerEvent() {
        cout << name << "::triggerEvent() ~ Firing event with: 42\n";
        eventFloat.fire(42.0f);
    }
};

struct Listener : EventListener<Listener> {
    string name;
    Listener(string _name)
        : name(_name) {
        cout << name << "()\n";
    }
    ~Listener() {
        cout << "~" << name << "()\n";
        //emitter.eventFloat.removeSubscriber(this); 
    }

    void gotEvent(float x) { cout << name << "::gotEvent hit with value: " << x << endl; }
};

int main() {
    // event source and listener unaware of each other's existence
    Publisher publisherA("publisherA");

    Listener listener1("listener1");
    listener1.connect(publisherA.eventFloat, &Listener::gotEvent);

    {
        Listener listener2("listener2");
        listener2.connect(publisherA.eventFloat, &Listener::gotEvent);

        publisherA.triggerEvent();
    }

    publisherA.triggerEvent();

    return 0;
}

Upvotes: 1

Larry Kollasch
Larry Kollasch

Reputation: 11

If it were not for objects not "knowing" their existence, you could simply code the side affect you want into a Listener virtual base destructor to de-register itself when it leaves scope.

Callback API is decidedly a "C"-like construct. To bridge to C++, you need to provide instance context along with the callback method. Emitter API to pass the void* opaque client context reference only as a parameter so it doesn't actually "know" or care about the client type, it just needs to pass back the same void* _t that was given in registration. This enables main() to register &listener1 "this" pointer as a reference.

Turn Listener::getEvent() into a "C"-style static method that accepts some void* pointer which it then casts into a Listener object and uses it to determine object existence before handling the event. A private, static std::set container would be handy for verifying. This safely completes the bridge into C++ land.

Upvotes: 1

Related Questions