Reputation: 30684
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
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
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