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.