Re: strange deadlock ...

[Michael Meeks <michael ximian com>]

On 17 Jul 2001, Owen Taylor wrote:
> > 	On the contrary, it will cripple anyone using threads and Gtk+ eg.
> > try adding a g_thread_init to gtk-demo, and then pressing "Dialog and
> > Message Boxes" and then any button [ to cause a re-entering of the glib
> > mainloop through gtk_dialog_run ].
> 
> But this is just a bug in gtk_dialog_run. gtk_dialog_run() knows that
> if g_thread_init() has called an the application isn't buggy, that it
> holds the GTK+ lock, so it should do:

	I beg to differ.

>   ri.loop = g_main_new (FALSE);
> 
> +  GDK_THREADS_LEAVE ()
> 
>   g_main_loop_run (ri.loop);
> 
> -  GDK_THREADS_ENTER ()

	Assuming you meant '+' not '-' here, or it's truly broken.

> The problem is that in the case of Bonobo usage, you have know idea
> whether the GDK lock is held or not.

	No - both cases are the same. You can't be sure what is on the
stack when you are called, it is true that in the case I mention that the
dialog_run is hit via. the handling of a gdk_event [1] and thus the lock
is held.

	But, there is no reason why this should be so, if we put the
dialog on a timeout, or if we do gtk_dialog_run inside our 'main'
function, then we will not have the GDK lock when we hit here, and then we
will try and drop the lock ... wow, amazing you can do:

	GTK_THREADS_LEAVE ();
	GTK_THREADS_LEAVE ();
	GTK_THREADS_LEAVE ();
	GTK_THREADS_LEAVE ();

	And get no "You're going to die" type warning whatsoever.

	Either way, you have dropped a lock that wasn't taken, then after
quitting the dialog, you do a GDK_THREADS_ENTER - which does in fact
re-instate the lock [ that it shouldn't own in fact ], and the next
gdk_event_prepare will deadlock.

	So - of course, perhaps throwing up dialogs on a timeout is
broken, perhaps in fact the solution to all locking problem is:

	if (!(was_locked = peek_lock_locked))
		lock_lock;

	do magic
	
	if (!was_locked)
		unlock_lock;

	Alternatively - perhaps the GDK_THREADS_ENTER / LEAVE scoping
needs re-thinking. Whilst I know nothing about the issues whatsoever I
can't resist suggesting a solution:

	Move the GTK_THREADS locks down to where they are actualy useful,
and ensure that they are not held for long periods of time as a matter of
course. So, eg.

static gboolean  
gdk_event_dispatch (GSource    *source,
		    GSourceFunc callback,
		    gpointer    user_data)
{
  GdkEvent *event;
 
  GDK_THREADS_ENTER ();

  gdk_events_queue();
  event = gdk_event_unqueue();

+  GDK_THREADS_LEAVE ();

  if (event)
    {
      if (gdk_event_func)
	(*gdk_event_func) (event, gdk_event_data);
      
+      GDK_THREADS_ENTER ();

      gdk_event_free (event);

+      GDK_THREADS_LEAVE ();
    }
  
-      GDK_THREADS_LEAVE ();

  return TRUE;
}

	Then in gtk_main_do_event we can do some more lock taking and
releasing - of course this will not have a pleasant performance impact. So
perhaps instead - just release the lock before we do the switch
event->type here, and take it again afterwards.

	And again, I know little about good practice with locking, but
things like this:

  if (g_main_is_running (main_loops->data))
    {
      GDK_THREADS_LEAVE ();
      g_main_run (loop);
      GDK_THREADS_ENTER ();
      gdk_flush ();
    }

	Looks like a recepie for disaster. 

	Then again, it is entirely possible that I've totaly misunderstood
any or all of Gtk+, Gdk, threads, locking, desirable granularity, the
penalty of lock taking, what a deadlock is etc. etc. so please be patient
with me.

	Regards,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot