Re: Glib 2.32.x, Win32 and threading

["Martin Schlemmer" <Martin Schlemmer nwu ac za>]

>>> On 7/4/2012 at 10:30 AM, Chris Vine <chris cvine freeserve co uk>
wrote:
> On Tue, 03 Jul 2012 20:59:11 +0200
> "Martin Schlemmer" <Martin Schlemmer nwu ac za> wrote:
>> Hi,
>> 
>> Bit long, sorry.
>> 
>> I build my own GTK+ stack with MinGW (TDM-4.6.1) and have been
using
>> 2.16 and Glib 2.28 (with Gtk2-Perl bindings mostly) for a long
time,
>> then switched to GTK+ 2.24 when that started to get stable enough.
>> Wanting to start with WebKit I switched to Glib 2.30 to be able to
>> use GObject-Introspection from Dieter's git repo.
>> 
>> This still worked fine, but needing later G-O-I interfaces from
>> WebKit, I tried to build 1.8 which needs Glib 2.31 or later, so I
>> switched to Glib 2.32.3, and this is where my problems started.
>> 
>> All threaded Gtk2-Perl apps locked up on startup and I initially
>> thought it was a Gtk2-Perl problem, but building the example from
the
>> FAQ:
>> 
>> http://developer.gnome.org/gtk-faq/stable/x481.html 
>> 
>> result in the same problem. The easiest way to reproduce it is to
>> start the example and then move the window which immediately
results
>> in a deadlock.
> 
> GTK+/GDK has never been thread-safe on Win32.  If the GDK global
lock
> appeared to work for you in the past you just got lucky.  From the
> documentation at
> http://developer.gnome.org/gdk/stable/gdk-Threads.html :
> 
> "GTK+ is "thread aware" but not thread safe * it provides a global
lock
> controlled by gdk_threads_enter()/gdk_threads_leave() which protects
> all use of GTK+. That is, only one thread can use GTK+ at any given
> time.
> 
> "Unfortunately the above holds with the X11 backend only. With the
Win32
> backend, GDK calls should not be attempted from multiple threads at
> all."
> 
> In due course
gdk_threads_init()/gdk_threads_enter()/gdk_threads_leave()
> are to be deprecated in the X11 backend, although that is not
relevant
> to your code.
> 
> Win32 users (and in due course X11 users) should use
> g_idle_add()/g_idle_add_full() to send event callbacks from worker
> threads to the gui thread which invoke the GTK+ functions you want
to
> call. With glib < 2.32, you will need to call g_thread_init()
> (g_thread_init() is not needed on glib >= 2.32 and is a no-op.) 
This
> also happens to be a much cleaner design than using the GDK global
lock.
> 
> The FAQ on this is, I agree, misleading - I should file a
documentation
> bug.
> 

I understand this, and that is why I modified the example to do exactly
that - use g_idle_add().

The problem currently as far as I can see it, is that when processing
events with the GDK global lock taken in gdk_event_dispatch(), as soon
as its a modal operation gets queued in
_gdk_win32_display_queue_events(), modal_timer_proc() gets called which
calls g_main_context_pending() and you eventually end up in
gdk_event_prepare() which tries to take the GDK global lock for a second
time in the same thread ...

Just to test, I removed any threads, and the problem persists with the
same backtrace (see below).

Anyhow, below patch (also attached) fixes it by not calling
_gdk_win32_display_queue_events() with the lock held, if anybody can
have a look if there are any locking problems I would appreciate it.

-----
diff --git a/gdk/win32/gdkevents-win32.c b/gdk/win32/gdkevents-win32.c
index 2bc7c12..3505fe2 100644
--- a/gdk/win32/gdkevents-win32.c
+++ b/gdk/win32/gdkevents-win32.c
@@ -3369,9 +3369,10 @@ gdk_event_dispatch (GSource     *source,
 {
   GdkEvent *event;
  
+  _gdk_win32_display_queue_events (_gdk_display);
+
   GDK_THREADS_ENTER ();
 
-  _gdk_win32_display_queue_events (_gdk_display);
   event = _gdk_event_unqueue (_gdk_display);
 
   if (event)
-----


Thanks,
Martin

-----
int main(int argc, char *argv[])
{
  GtkWidget *window;
  GtkWidget *label;

  gdk_threads_init();
  gtk_init(&argc, &argv);

  window = gtk_window_new(GTK_WINDOW_TOPLEVEL);

  g_signal_connect(window, "destroy",
		   G_CALLBACK(destroy), NULL);

  gtk_container_set_border_width(GTK_CONTAINER (window), 10);

  label = gtk_label_new("And now for something completely different
...");
  gtk_container_add(GTK_CONTAINER(window), label);

  gtk_widget_show(label);
  gtk_widget_show (window);

  gdk_threads_enter();
  gtk_main();
  gdk_threads_leave();

  return 0;
}
-----



Vrywaringsklousule / Disclaimer: 
http://www.nwu.ac.za/it/gov-man/disclaimer.html 

diff --git a/gdk/win32/gdkevents-win32.c b/gdk/win32/gdkevents-win32.c
index 2bc7c12..3505fe2 100644
--- a/gdk/win32/gdkevents-win32.c
+++ b/gdk/win32/gdkevents-win32.c
@@ -3369,9 +3369,10 @@ gdk_event_dispatch (GSource     *source,
 {
   GdkEvent *event;
  
+  _gdk_win32_display_queue_events (_gdk_display);
+
   GDK_THREADS_ENTER ();
 
-  _gdk_win32_display_queue_events (_gdk_display);
   event = _gdk_event_unqueue (_gdk_display);
 
   if (event)