Frank Mori Hess wrote:
One solution is to overload the signal::connect function with another version that accepts an additional weak_ptr<void> argument, which points to the object whose member function is being used as a slot. The signal could try to convert the weak_ptr into a shared_ptr<void> whenever it runs the slot. Then the signal can either clean up the connection if the object is dead, or be sure the object won't die while the slot is running. The disadvantage, of course, is it forces people to use shared_ptrs to manage the lifetimes of objects they want to use thread-safe automatic connection management with. But that seems preferable to offering no means of thread-safe automatic connection management at all.
I see two problems with that (well, three, but I'll leave performance issues out for now ;)): 1. Cleanup only takes place when the signal is actually called, so it may end up tracking a large amount of pointers in a situation where the connection and observer destruction frequency is much higher than the signal call rate. I'm using it to notify thread local storage owners of threads exiting, for example. You can imagine that this won't work well for a signal that is very likely to be called only once at application shutdown, while an arbitrary amount of slots can be connected and disconnected in the meantime. If I'm not mistaken, your current implementation has the same problem for manual disconnections. 2. Using an overload only allows tracking of a fixed number of objects. You're currently just considering the situation in which the object to which the member function is bound to can be tracked, but as far as I understand it, signals::trackable allows you to monitor every bound argument, including ordinary call parameters. Both problems can be solved, of course, so please just see this as a reminder. The overall solution looks good.
I imagine it could be. Optimization wasn't high on my priority list while writing it, I was aiming more for correctness, followed by ease of implementation.
I agree.
I'm not following what you're suggesting. Would you spell it out in more detail, or even better say it with a patch?
Patch attached. Regards Timmo Stange 46,62c46,47 < void disconnect() < { < boost::mutex::scoped_lock lock(mutex); < do_disconnect(); < } < bool connected() const < { < boost::mutex::scoped_lock lock(mutex); < do_connected(); < } < < private: < virtual void do_disconnect() = 0; < virtual bool do_connected() const = 0; < < // mutex should be locked when slot is called, to prevent race with disconnect() < mutable boost::mutex mutex; ---
virtual void disconnect() = 0; virtual bool connected() const = 0;
73,75c58 < < private: < virtual void do_disconnect() ---
virtual void disconnect()
76a60
boost::mutex::scoped_lock lock(mutex);
79c63 < virtual bool do_connected() const ---
virtual bool connected() const
80a65
boost::mutex::scoped_lock lock(mutex);
82a68
// mutex should be locked when slot is called, to prevent race with disconnect()
83a70
mutable boost::mutex mutex;