[sigc] Glib::Thread::create() thread safety

[Chris Vine <chris cvine freeserve co uk>]

According to the libsigc++ documentation (I think), if a slot created
via sigc::mem_fun represents a non-static method of a class object
derived from sigc::trackable, the slot will automatically be invalidated
and disconnected if that object is destroyed.

If that is correct, it would be achieved on slot creation by a call to
sigc::trackable::add_destroy_notify_callback() to register the slot with
the relevant sigc::trackable target object belonging to the object whose
method is bound to the slot.  That function manipulates the
std::list<sigc::trackable_callback> object held by the sigc::trackable
object's sigc::trackable_callback_list member (it adds the relevant
trackable_callback to the list using
std::list<sigc::trackable_callback>::push_back()).  On slot destruction
the reverse happens - a call would be made to
sigc::trackable::remove_destroy_notify_callback(), which removes the
relevant trackable_callback from the list by calling
std::list<sigc::trackable_callback>::erase().

If (see [1] below) slots do behave in this way, it would follow that
thread creation in glibmm using Glib::Thread::create() is not thread
safe where the callback invoked is a non-static method of a class
derived from sigc::trackable.  This is because the slot object to be
executed by the new thread is created on freestore in
Glib::Thread::create() (thus manipulating the trackable object's list)
but destroyed in the new thread in call_thread_entry_slot() (see 'delete
slot' in the penultimate line of that function in glibmm/thread.cc), so
also manipulating the std::list object without mutex protection in a
different thread.

That is not of itself problematic since clearly the deregistration of
the slot in call_thread_entry_slot() cannot occur concurrently with its
registration.  However, it could occur at the same time that the
original thread is creating new slots for the same target object whose
method was bound to the slot: in that case, there would be undefined
behaviour.  (call_thread_entry_slot() also creates a temporary
sigc::slot object in the call to
(*static_cast<sigc::slot<void>*>(slot))() which dispatches the thread
callback, also giving rise to the potential for concurrency with other
manipulations of the std::list object by the original thread.)

Unless there is something wrong with this analysis I propose to post a
bug on bugzilla, but before doing so could someone else take a look?  I
have the feeling that it is unlikely that something like this would have
easily escaped into the library.  See also comment #5 at
http://bugzilla.gnome.org/show_bug.cgi?id=396958 , raising a similar
issue.

Because of [1] below I am copying this to the libsigc++ list - sorry if
that causes inconvenience.

Chris

[1] I cannot identify the code which in fact does this.  The constructor
of sigc::typed_slot_rep calls functor sigc::slot_do_bind (via helper
sigc::visit_each_type).  operator() of the slot_do_bind functor calls
sigc::trackable::add_destroy_notify_callback(), and likewise the
destructor calls sigc::trackable::remove_destroy_notify_callback().
However those calls seem to be to the sigc::trackable object from which
sigc::slot_rep is derived (a 'this' pointer is passed), which is not as
far as I can see related to the sigc::trackable object of the target
class whose method has been bound to the slot.  However, at that point
my powers of analysis cease because I cannot penetrate the code any
further.