Re: thread safe gtk?

[Owen Taylor <otaylor gtk org>]

Rob Browning <rlb@cs.utexas.edu> writes:

> Owen Taylor <otaylor@gtk.org> writes:
> 
> > The problem is, gtk_main_iteration() will just block on the select()
> > until an event happens, without regard to the desires of the other
> > threads. I don't think you can get this sort of thing working
> > properly without basically modifying the main loop. (One approach
> > which avoids putting threading dependent code into GTK+ is to allow
> > the application to supply a replacemnt for select())
> 
> Good point.  I wasn't very familiar with what's going on in gtk_main,
> but I just had a look, and I agree, we have to have some *minor*
> hackery in gdk_event_wait.  Do you think this would be acceptable
> upstream?
> 
> As a proposal, here's one potential solution (essentially like yours).
> First of all, we need to move everything into gdk, so gdk won't depend
> on gtk.  We can still have gtk macro wrappers if we like...
> 
> Then a minor modification to gdk.c:gdk_event_wait()
> 
>   gdk_event_wait() {
>     ...
> #   ifdef GDK_THREADS_AWARE  
>     if(gdk_using_threads) gdk_threads_leave();
> #   endif
>     nfd = select (max_input+1, &readfds, &writefds, &exceptfds, timerp);
> #   ifdef GDK_THREADS_AWARE  
>     gdk_threads_enter();
> #   endif
>     ...
>   }
> 
> And an equally minor modification to gdk_threads_init():
> 
>   void
>   gtk_threads_init (void)
>   {
>     gdk_using_threads = TRUE;
>   }
> 
> No more pipes, and exactly the behavior we want.  If the user doesn't
> call it, there's no penalty.  Even if they do, there's very little.

Definitely more efficient than the pipe stuff. (But the pipe stuff
is neat because you can get things working without any modifications
to the toolkit at all).

I think something like this is essentially right. I have a
few questions about the interface. These come from two 
separate directions: 

 I) How can the modifications be most generally useful?
   A) Main loop changes:
     1) For normal threading?
     2) For cooperative threading (requires replacing select())?
     3) For programs that want to replace the select() for other
        reasons?
   B) Should the thread primitives (lock/unlock), be changeable:
     1) At compile time? (should we think about things other 
        than pthreads)
     2) At run time? (Program supplies a pointer to a lock/unlock
        functions, or to create/destroy/lock/unlock mutex functions)

 II) Preparing for more extensive threading in the future.
   Does anything need to be done now to prepare for, in the future:

   A) Adding locks to every GTK+ entry point.
   B) Making things like gtk_widget_push_colormap() thread specific
   D) Making the locks more fine-grained. [ Probably a high
      trouble, low gain activity ]

With all that in mind, it is easy to come up with lots of 
different interfaces, from your simple:

 gdk_threads_init()
 gdk_threads_enter()
 gdk_threads_leave()

./configure --enable-threads[=pthreads]

or simpler:
 
 void gdk_set_select(gint (*f) (int numfds, fd_set *readfds, 
                                 fd_set *writefds, fd_set *exceptfds, 
                                 struct timeval * timeout));

To considerably more complex alternatives:
 
 typedef struct {
   gpointer (*create_mutex) (void);
   void (*lock_mutex) (gpointer);
   void (*unlock_mutex) (gpointer);
   void (*destroy_mutex) (gpointer);

   gint (*select) (int numfds, fd_set *readfds, fd_set *writefds ,
	           fd_set *exceptfds, struct timeval * timeout);

   [ more functions ? ]
 } GtkThreadFuncs;
 
 gdk_threads_set_funcs (GtkThreadFuncs *funcs);
 gdk_threads_init();  /* Defaults to pthreads */
 gdk_threads_enter();
 gdk_threads_leave();

The first has the major advantage that it is simple, and largely a
complete solution. It is also mostly orthogonal to a complicated
"change everything at runtime" solution like the third one, so that
could be added later.

So I don't really see any problem with adding it to the 1.1
branch of GTK+ immediately. 

One minor question is whether gtk_threads_enter() should be
recursive. That is possible for LinuxThreads (and pthreads in
general?), though it slows things down a bit. But it might also make
things easier to program.

> > I'm not sure how important fairness is here. Hopefully, a meaningful
> > threaded application would be doing enough stuff without the
> > lock so that things should balance out fairly well. (My test
> > program suffers fairly badly from fairness, but it isn't
> > representative).
> 
> The problem here is that (I think) two threads can dominate the lock.
> If they are both trying to get it in tight loops, and there a third
> thread that wants it too, the third thread can sit at the back of the
> mutex queue for an arbitrarily long time while the LIFO scheduling
> just lets the two other threads hand off to each other.  I'm not sure
> we'd have this problem, but we should investigate, and if it's a
> problem, prevent it, so that someone else doesn't have to figure it
> out the hard way.

I took a look at the LinuxThreads code, and it actually is FIFO.
The problem you are seeing here is something else:

 If a single thread does a 'unlock(); lock()' then it will
 almost certainly get the lock back, instead of a thread
 waiting on the lock that gets woken up after the unlock().
 
Fixing this without is hard because if you wait for the
other thread, you pay a substantial penalty in the case
where a thread unlock()'s and goes off and does something
else - which is how a threaded program should work anyways.
(This is discussed in the LinuxThreads FAQ.)

Regards,
                                        Owen