Re: [Boost-users] [signals2][review] The review of the signals2 library (formerly thread_safe_signals) begins today, Nov 1st

Subject: Re: [Boost-users] [signals2][review] The review of the signals2 library (formerly thread_safe_signals) begins today, Nov 1st
From: Nat Goodspeed (nat_at_[hidden])
Date: 2009-02-11 15:08:08

Sorry for the long delay... I got behind on the Boost list and am only 
gradually catching up.  :-(

>> Johan Råde wrote:

>>> Frank also mentioned the possibility of adding
>>> a thread safe version of boost::trackable
>>> based on boost::enable_shared_from_this.

Frank Mori Hess wrote:

> I have added a thread-UNsafe boost::signals2::trackable to svn to ease 
> porting of single-threaded Boost.Signals code that doesn't need to be made 
> thread-safe.  

That will be useful transitionally if we can count on a thread-safe 
boost::signals2::trackable arriving later. If signals2::trackable would 
only ever be thread-unsafe, it's not useful to us.

I recently introduced into our code some machinery based on 
boost::signal. Others are concurrently working on multithreading. I 
don't want my new functionality to become a source of difficult race 
bugs -- or even to be /perceived/ that way. I want to avoid a label of: 
"this mechanism is thread-unsafe, avoid it in all new code."

Accordingly, I've just replaced boost::signal with 
boost::signals2::signal, and so forth, throughout our code.

There were several existing uses of boost::trackable. I've coded around 
them in several ways, as described below. Since I didn't introduce them, 
I don't know the author's intent.

> Would you give a little more detail on what parts of boost::trackable 
> porting you've found the most painful?  

Those of you uninterested in details can skip the rest. :-)

My Holler class contains a boost::signals2::signal<void(const Data&)>. 
For simple usage ignoring lifespan issues (e.g. connecting a free 
function), I have a Holler::listen(const slot_type&) method. It's nice 
that with boost::trackable, the same listen() method Just Works.

With an instance 'smartptr' of ListenerClass derived from 
boost::trackable, I'd like to write something like:

     holler.listen(boost::bind(&ListenerClass::method, smartptr, _1));

If my listen() method were able to tease apart the object returned from 
bind(), it could detect the case of a shared_ptr<boost::trackable subclass>.

Since I don't know how to do that, I've introduced a number of 
alternative ways to get disconnect-on-destruction. I don't yet know 
which, if any of them, will become the prevalent idiom. None is as 
easy/foolproof as boost::trackable, since each requires the caller to 
explicitly request connection management.

Maybe I'm overlooking something that would make my life much easier -- 
suggestions welcome!

1. I've introduced a template Holler::listen() overload like this:

template <class CLASS, typename POINTEE>
connection listen(void (CLASS::*method)(const Data&),
                   const boost::shared_ptr<POINTEE>& pointer);

Thus, instead of writing:

     holler.listen(boost::bind(&SomeClass::method, ptr, _1));

you'd write:

     holler.listen(&SomeClass::method, ptr);

This method instantiates a slot_type object, passes 'pointer' to its 
track() method and then calls the other listen() overload:

     slot_type listener(boost::bind(method, pointer.get(), _1));
     // n.b. gcc 3.3 doesn't like listener(method, pointer.get(), _1)
     listener.track(pointer);
     return listen(listener);

2. For transient objects not managed by shared_ptr, I've also introduced 
a Trackable base class containing
    boost::ptr_vector<boost::signals2::scoped_connection> mConnections;

The Trackable::track(const connection& c) method does this:
     mConnections.push_back(new scoped_connection(c));

So destroying a Trackable subclass object disconnects any connections 
passed to track().

Again, if my Holler::listen() method could detect a bind() involving a 
TrackableSubclass*, it could implicitly call Trackable::track().

2a. You can explicitly engage track() using syntax such as:

listenobj.track(holler.listen(boost::bind(&TrackableSubclass::method,
                                            listenobj,
                                            _1)));

Trackable also defines a suite of listenTo() methods:

2b. connection listenTo(Holler&, const slot_type&);

2c. // harmonious with the Holler::listen() overload
     template <class CLASS, typename POINTER>
     connection listenTo(Holler&,
                         void (CLASS::*method)(const Data&),
                         const POINTER& pointer);
     // doesn't need to be boost::shared_ptr because we're
     // using Trackable::track() rather than slot_type::track()

2d. // for a Trackable subclass object to bind one of its own methods
     template <class CLASS>
     connection listenTo(Holler&,
                         void (CLASS::*method)(const Data&));

3. One of my colleagues has a class that (with my changes) now also 
wraps a boost::signals2::signal. He strongly dislikes the variation 
between my listen(const slot_type&) and listen(method ptr, shared_ptr) 
methods, so he's changed his own connectFor() method as follows:

     template <typename POINTER>
     connection connectFor(const slot_type& slot, const POINTER& ptr)
     {
         connection c(mSignal.connect(slot));
         Trackable* isTrackable(dynamic_cast<Trackable*>(ptr));
         if (isTrackable)
         {
             isTrackable->track(c);
         }
         return c;
     }

(Yes, this could be made cleverer to notice when POINTER is actually a 
boost::shared_ptr<SOMETHING>. Next iteration.)

So you subscribe to his class with a uniform call such as:

     obj.connectFor(boost::bind(&SomeClass::method, aPointer, _1),
                    aPointer);

But then you have to pass a NULL pointer for the case of a free function 
-- leaving open the possibility that someone will carelessly 
copy-and-paste the wrong example instance and pass NULL with a 
boost::bind() expression.

As I said above, every one of these approaches requires coding something 
other than the intuitive listen(boost::bind(etc.)) call, meaning you 
must know when you need to request connection management. The variety of 
options is undoubtedly a bad thing: how to choose? You must know way 
more than you should about what I had in mind.

A thread-safe boost::signals2::trackable mechanism would allow us to 
drop back to a single universal call:

     holler.listen(boost::bind(&SomeClass::method, ptr, _1));

with connection management automatically engaged as appropriate.

A transitional thread-unsafe boost::signals2::trackable would be tenable 
for now...

but if boost::signals2::trackable were only ever going to be 
thread-unsafe, I couldn't encourage people to use it: we'd be 
responsible for propagating new thread-unsafety through an application 
to which we're even now adding threads.