Merge lp:~vanvugt/lightdm/fix-1192051 into lp:~mir-team/lightdm/unity

[Daniel van Vugt]

seat-unity: Wait for Plymouth to complete the "deactivate" command. Otherwise
we risk fighting for the VT device and Plymouth's error handling is not robust
enough to handle that, making it spin at 100% CPU and never deactivating.
(LP: #1192051)

I'm sure some kind of robustness fix could be done in plymouth instead, but:

1. I don't yet understand the full flow of this bug from lightdm to plymouth. Other than plymouth is spinning on an fd of "/dev/tty7" with errno==EIO. And waiting for the plymouth deactivate to properly complete (hence drmDropMaster), avoids the bug.

2. Our PPAs don't contain plymouth so packaging the fix there would take significantly more effort.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1618
http://jenkins.qa.ubuntu.com/job/mir-team-lightdm-unity-ci/20/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-lightdm-unity-saucy-amd64-ci/1

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-lightdm-unity-ci/20/rebuild

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

Hacky, but will work for now.

review: Approve

[Daniel van Vugt (vanvugt)]

I agree it's hacky. And I spent a couple of extra hours trying to find a way to make it less so. This is the least of the evils.

=== modified file 'src/plymouth.c'
--- src/plymouth.c	2011-09-07 05:37:55 +0000
+++ src/plymouth.c	2013-06-19 13:42:27 +0000
@@ -81,10 +81,30 @@
 }
 
 void
-plymouth_deactivate (void)
+plymouth_deactivate (gboolean wait)
 {
+    gint attempt = 1;
     is_active = FALSE;
-    plymouth_run_command ("deactivate", NULL);
+
+    /*
+     * The "plymouth" command has no option like --wait-for-deactivation so
+     * we have to wait in a loop.
+     */
+    do
+    {
+        g_debug("Deactivating Plymouth (attempt #%d)", attempt);
+        attempt++;
+        plymouth_run_command ("deactivate", NULL);
+
+        if (!wait)
+            return;
+
+        sleep (1);
+        have_checked_active_vt = FALSE;
+    } while (plymouth_has_active_vt () && attempt <= 60);
+
+    if (has_active_vt)
+        g_debug("Failed to deactivate Plymouth.");
 }
 
 void

=== modified file 'src/plymouth.h'
--- src/plymouth.h	2013-04-23 03:07:03 +0000
+++ src/plymouth.h	2013-06-19 13:42:27 +0000
@@ -22,7 +22,7 @@
 
 gboolean plymouth_has_active_vt (void);
 
-void plymouth_deactivate (void);
+void plymouth_deactivate (gboolean wait);
 
 void plymouth_quit (gboolean retain_splash);
 

=== modified file 'src/seat-unity.c'
--- src/seat-unity.c	2013-06-19 04:32:53 +0000
+++ src/seat-unity.c	2013-06-19 13:42:27 +0000
@@ -305,7 +305,7 @@
         {
             g_debug ("Compositor will replace Plymouth");
             SEAT_UNITY (seat)->priv->vt = active_vt;
-            plymouth_deactivate ();
+            plymouth_deactivate (TRUE);
         }
         else
             g_debug ("Plymouth is running on VT %d, but this is less than the configured minimum of %d so not replacing it", active_vt, vt_get_min ());

=== modified file 'src/xserver-local.c'
--- src/xserver-local.c	2013-06-17 23:38:35 +0000
+++ src/xserver-local.c	2013-06-19 13:42:27 +0000
@@ -170,7 +170,7 @@
             g_debug ("X server %s will replace Plymouth", xserver_get_address (XSERVER (self)));
             self->priv->replacing_plymouth = TRUE;
             self->priv->vt = active_vt;
-            plymouth_deactivate ();
+            plymouth_deactivate (FALSE);
         }
         else
             g_debug ("Plymouth is running on VT %d, but this is less than the configured minimum of %d so not replacing it", active_vt, vt_get_min ());