glibmm thread safety [was: Re: Adding custom GDK events]

[Daniel Elstner <daniel kitta googlemail com>]

Am Sonntag, den 14.01.2007, 23:49 +0000 schrieb Chris Vine:
> On Sunday 14 January 2007 00:38, Chris Vine wrote:
> [snip]
> > As I think you are saying, this does not in itself ensure memory visibility
> > of the callback function pointer or data argument of the idle callback (or
> > any other data passed between the threads concerned) and if visibility on
> > multi-processor systems is required then some form of memory
> > synchronisation (fences or barriers) will be required, typically by means
> > of mutex locking of the data.

Oh, this isn't only necessary on multiprocessor systems.  I remember
hitting memory visibility problems in my very first threaded project,
and that was on UP.  The program didn't crash in the debugger, and only
if a certain piece of code used memcpy() instead of memmove().  After
re-reading the related chapters in Butenhof's book about three times, I
finally understood that the memcpy() was correct and my threading code
was wrong.  This was one of these situations where the program logic
dictated a certain chronological order, but nonetheless memory
visibility wasn't satisfied.

> > In practice however I suspect this will be achieved since I am pretty sure
> > (although this perhaps should be checked and I will do so next week ...
> [snip]

Cool.

> Each GMainContext object has a mutex associated with it which is used to lock 
> main context operations.  g_idle_add_full() creates the new idle source 
> object and then calls g_source_set_callback(), which assigns (amongst other 
> things) the function pointer and data arguments to the data members of the 
> relevant idle source object.  At this point the memory representing the 
> function pointer and data argument is unsynchronised.  However it follows 
> this with a call to g_source_attach() which attaches the idle source object 
> to the relevant GMainContext object under the mutex lock for that main 
> context (an acquire and a release).
> 
> When the data members of the idle source object are read for dispatching, 
> g_main_context_dispatch(), which calls g_main_dispatch(), does so under the 
> same mutex lock, with an acquire and a release.  Therefore, when the main 
> loop thread reads these data members, there has been (a) a release operation 
> by the signalling thread, and (b) an acquire operation on the same 
> synchronisation object by the reading thread.  Accordingly the memory of the 
> reading thread will be synchronised with that of the signalling thread to a 
> condition no earlier than the release instruction being executed, and no out 
> of order execution of the write will be permitted beyond the point of the 
> acquire or the point of synchronisation, assuming a platform with these 
> minimum-level thread safety guarantees.  (Actually Posix, in section 4.10, 
> General Concepts, Memory Synchronization, gives stronger guarantees than 
> that.)

OK.  I misinterpreted one of the rules in Butenhof's book to be much
more strict than it actually is:

2. Whatever memory values a thread can see when it unlocks a mutex, either 
directly or by waiting on a condition variable, can also be seen by any 
thread that later locks the same mutex. Again, data written after the mutex 
is unlocked may not necessarily be seen by the thread that locks the mutex, 
even if the write occurs before the lock.

That apparently doesn't imply that the thread needs to have the mutex
locked while writing the data, as long as it locks and then unlocks it
before the other thread can get there.

> You know glibmm better than me, but it seems to me that the same would apply 
> to that.  Glib::signal_idle().connect() will create the idle source, and then 
> call Glib::SignalIdle::connect() on it, which will invoke g_source_attach().  
> The only thing I notice is that Glib::SignalIdle::connect(), after the call 
> to g_source_attach(), calls Glib::SourceConnectionNode::install().  I do not 
> know what that does, but if it is important to dispatch of the slot in the 
> main loop by the relevant GMainContext object you could always put 
> LOCK_CONTEXT(context_)/UNLOCK_CONTEXT(context_) around it (or if it makes 
> sense to put the call to Glib::SourceConnectionNode::install() before the 
> call to g_source_attach(), put it before the call to g_source_attach()).

inline
void SourceConnectionNode::install(GSource* source)
{
  source_ = source;
}

It's trivial, and can be moved somewhere before the g_source_attach()
without harm.  In fact, it should be moved, as it writes data that might
be read by another thread.

However, I can still see one possible show-stopper: sigc++.  Well,
contrary to what I feared at first, explicitly copying a sigc::slot
performs a deep copy.  In particular, this comment relieved me
(note that sigc::slot_rep is a subclass of sigc::trackable):

/* Don't copy the notification list.
   The objects watching src don't need to be notified when the new
object dies. */
trackable::trackable(const trackable& /*src*/)
: callback_list_(0)
{}

Now to the bad news.  Glib::signal_idle().connect() returns a
sigc::connection object.

connection::connection(slot_base& sl)
: slot_(&sl)
{
  //Let the connection forget about the signal handler when the handler
object dies:
  slot_->add_destroy_notify_callback(this, &notify);
}

connection::~connection()
{
  if (slot_)
    slot_->remove_destroy_notify_callback(this);
}

Doomed, I'd say.  At least the call to remove_destroy_notify_callback()
after the GSource mutex has been unlocked is unavoidable with the
current API.  Maybe we'll be able to play some tricks and put something
entirely different into the sigc::connection object.  A special-purpose
slot which holds a connection ID or something, and goes through the
GMainLoop/GSource interface to destroy the callback slot.

Unfortunately, I don't really have that much time at hand these days.

--Daniel