Merge lp:~afrantzis/unity-system-compositor/no-external-spinner-zombie-processes-sigchld-with-test into lp:unity-system-compositor

[Alexandros Frantzis]

Don't leave zombie spinner processes

Don't leave zombie spinner processes

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:224
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/242/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-amd64-ci/30
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/30
        deb: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/30/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-i386-ci/30

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/242/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:226
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/243/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-amd64-ci/31
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/31
        deb: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-armhf-ci/31/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-wily-i386-ci/31

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/243/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

lgtm

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/external_spinner.cpp'
--- src/external_spinner.cpp	2015-04-01 17:06:16 +0000
+++ src/external_spinner.cpp	2015-07-10 13:38:55 +0000
@@ -20,6 +20,18 @@
 
 #include <unistd.h>
 #include <signal.h>
+#include <sys/wait.h>
+
+namespace
+{
+
+void wait_for_child(int)
+{
+    while (waitpid(-1, nullptr, WNOHANG) > 0)
+        continue;
+}
+
+}
 
 usc::ExternalSpinner::ExternalSpinner(
     std::string const& executable,
@@ -28,6 +40,11 @@
       mir_socket{mir_socket},
       spinner_pid{0}
 {
+    struct sigaction sa;
+    sigfillset(&sa.sa_mask);
+    sa.sa_handler = wait_for_child;
+    sa.sa_flags = 0;
+    sigaction(SIGCHLD, &sa, nullptr);
 }
 
 usc::ExternalSpinner::~ExternalSpinner()

=== modified file 'tests/integration-tests/test_external_spinner.cpp'
--- tests/integration-tests/test_external_spinner.cpp	2015-07-10 13:38:55 +0000
+++ tests/integration-tests/test_external_spinner.cpp	2015-07-10 13:38:55 +0000
@@ -71,6 +71,16 @@
     return pids;
 }
 
+bool is_zombie(pid_t pid)
+{
+    std::ifstream stat("/proc/" + std::to_string(pid) + "/stat");
+
+    std::stringstream ss;
+    ss << stat.rdbuf();
+
+    return ss.str().find(" Z ") != std::string::npos;
+}
+
 struct AnExternalSpinner : testing::Test
 {
     std::vector<pid_t> spinner_pids()
@@ -170,3 +180,21 @@
 
     EXPECT_THAT(environment_of_spinner(), Contains("MIR_SOCKET=" + mir_socket));
 }
+
+TEST_F(AnExternalSpinner, does_not_leave_zombie_process)
+{
+    using namespace testing;
+
+    spinner.ensure_running();
+    auto const spinner_pid = spinner_pids()[0];
+    spinner.kill();
+
+    wait_for_spinner_to_terminate();
+
+    // Wait a bit for zombie to be reaped by parent
+    bool const spinner_is_not_zombie = usc::test::spin_wait_for_condition_or_timeout(
+        [spinner_pid] { return !is_zombie(spinner_pid); },
+        timeout);
+
+    EXPECT_TRUE(spinner_is_not_zombie);
+}