Re: [Boost-users] [signals] Automatic connection management and MT

From: Timmo Stange (ts_at_[hidden])
Date: 2007-02-01 18:37:23

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;