Merge lp:~attente/unity-gtk-module/1258669 into lp:unity-gtk-module/14.04

[William Hua]

Dispatch gtk_menu_item_activate () in a GDK idle.

Dispatch gtk_menu_item_activate () in a GDK idle.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:315
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/28/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/14
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/14
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/14/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/28/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Probably would be best to add a comment to this describing why the activation has to be deferred to an idle timeout? I'm not saying this doesn't solve the bug, but it's not clear to me why it does... :)

review: Needs Information

[lp:~attente/unity-gtk-module/1258669]

316. By William Hua on 2014-03-06

Describe why the deadlock occurs.

[William Hua (attente)]

Thanks Charles, to be honest, it's not clear to me what the culprit is. But I've added a comment with as much information as I know, with Ryan's help on the speculation. Hopefully someone smarter comes along and can tell us why exactly it happens...

It definitely does fix the problem though :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:316
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/31/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/17
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/17
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/17/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/31/rebuild

review: Approve (continuous-integration)

[lp:~attente/unity-gtk-module/1258669]

317. By William Hua on 2014-03-06

Merge trunk.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:317
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/32/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/18
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/18/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/32/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Thanks Will :)

review: Approve

=== modified file 'lib/unity-gtk-menu-shell.c'
--- lib/unity-gtk-menu-shell.c	2014-03-05 17:17:35 +0000
+++ lib/unity-gtk-menu-shell.c	2014-03-06 18:56:31 +0000
@@ -93,6 +93,16 @@
   return !g_sequence_iter_is_end (iter) && g_sequence_get_uint (iter) <= i ? iter : NULL;
 }
 
+static gboolean
+gtk_menu_item_handle_idle_activate (gpointer user_data)
+{
+  g_return_val_if_fail (GTK_IS_MENU_ITEM (user_data), G_SOURCE_REMOVE);
+
+  gtk_menu_item_activate (user_data);
+
+  return G_SOURCE_REMOVE;
+}
+
 static GPtrArray *
 unity_gtk_menu_shell_get_items (UnityGtkMenuShell *shell)
 {
@@ -1027,7 +1037,24 @@
       if (GTK_IS_MENU (shell->menu_shell))
         gtk_menu_set_active (GTK_MENU (shell->menu_shell), item->item_index);
 
-      gtk_menu_item_activate (item->menu_item);
+      /*
+       * We dispatch the menu item activation in an idle to fix LP: #1258669.
+       *
+       * We get a deadlock when the menu item is activated if something like
+       * gtk_dialog_run () is called. gtk_dialog_run () releases the GDK lock
+       * just before starting its own main loop, and tries to re-acquire it
+       * once it terminates. For whatever reason, a direct call to
+       * gtk_menu_item_activate () here causes the GDK lock to be acquired
+       * before gtk_dialog_run () tries to acquire it, whereas dispatching it
+       * using gdk_threads_add_idle_full () seems to cleanly acquire the lock
+       * once only at the beginning, preventing the deadlock.
+       *
+       * Suspicion is that this was executing during the main context
+       * iteration of gtk_main_iteration (), which grabs the GDK lock
+       * immediately after. But it's still not clear how that's possible....
+       */
+
+      gdk_threads_add_idle_full (G_PRIORITY_DEFAULT_IDLE, gtk_menu_item_handle_idle_activate, g_object_ref (item->menu_item), g_object_unref);
     }
 }