Merge lp:~thomas-voss/unity-mir/refactor-process-group-operations-to-rely-on-process-cpp into lp:unity-mir

[Thomas Voß]

Refactor sigstop'ing and sigcont'ing of application process groups to rely on process-cpp.

Refactor sigstop'ing and sigcont'ing of application process groups to rely on process-cpp.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:144
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/155/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/16/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/16/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/16/console

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

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/unity-mir/refactor-process-group-operations-to-rely-on-process-cpp]

145. By Thomas Voß on 2013-12-05

Adjust to namespace core.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:145
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/174/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/37/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/38/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/37/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/174/rebuild

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/unity-mir/refactor-process-group-operations-to-rely-on-process-cpp]

146. By Thomas Voß on 2013-12-05

Mergeing.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:146
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/175/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/38/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/39/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/38/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/175/rebuild

review: Needs Fixing (continuous-integration)

[lp:~thomas-voss/unity-mir/refactor-process-group-operations-to-rely-on-process-cpp]

147. By Thomas Voß on 2013-12-09

[ Gerry Boland ]
* Unity.Application is not a shared library, but a plugin. (LP:
  #1256014)
* Implement preStart callback added to upstart-app-launch-2. (LP:
  #1243665)
[ Ubuntu daily release ]
* Automatic snapshot from revision 157
[ Gerry Boland ]
* Install ServerStatusListener to be notified of mir server start, pause
  and resume. Use start notification to send SIGSTOP signal to upstart,
  so it knows mir is ready for other clients.
* Bump version number
* InputArea: don't use lambda function as slot, can cause crash on
  shutdown Using lambda function as slot can introduce crash as the
  slot's object deletion is not registered unlike with traditional
  signal/slot connections. As a result, on signal emission, the lambda
  can be called on a deleted object. (LP: #1253685)
* Misc fixes for Mir 0.1.2 support. (LP: #1254986)
[ Alan Griffiths ]
* ApplicationSession is a Mir implementation class that shouldn't be
  used by unity-mir, use shell::session instead.
[ Kevin Gunn ]
* mir server abi and api broke, updating dependency to deb 0.1.1.
* update mir deb build dep to 0.1.2
[ Victor Thompson ]
* Fix mir to not suspend the music-app, or any other app granted a
  lifecycle exception, when switching between apps. (LP: #1241185,
  #1241045)
[ Daniel van Vugt ]
* Force the screen to redraw after turning the display back on (LP:
  #1255045). Also stop the compositor when the screen is off.
  Otherwise it will spin in the background, eating battery. (LP:
  #1255045)
[ Ubuntu daily release ]
* Automatic snapshot from revision 154
* Cherry-pick upstream patch to avoid Unity8 crashing on stop
  (LP: #1253685)
[ Gerry Boland ]
* On MirSurface destruction, any InputAreas on that surface will be
  notified and remove their links to that surface. Fixes crash
  bug:1243444. (LP: #1243444)
[ Alan Griffiths ]
* Remove dependency on mir::shell::SessionManager.
* Remove dependency on mir::surfaces::SurfaceController.
* Remove dependency on msh::OrganisingSurfaceFactory.
* Avoid relying on an explicit Mir typename that changes in a
  refactoring coming soon.
* Do all the hacky surface creation customization in
  SurfaceFactory::create_surface() - and don't mention SurfaceBuilder
  at all. (That will allow Mir to get rid of that interface.).
[ Albert Astals ]
* Don't include the QtQml megaheader Include the one we really need.
[ Ubuntu daily release ]
* Automatic snapshot from revision 144

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:147
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/181/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/44/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/45/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/44/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/181/rebuild

review: Needs Fixing (continuous-integration)

[Michał Sawicz (saviq)]

"We do assume" → "We assume"

What's the reason to initialise process with invalid? Doesn't seem like it'd ever be accessed if invalid anyway?

Here, too, feels like it'd be a good time to start auto-testing this.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:147
http://jenkins.qa.ubuntu.com/job/unity-mir-ci/191/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-amd64-ci/54/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-armhf-ci/55/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-mir-trusty-i386-ci/54/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-mir-ci/191/rebuild

review: Needs Fixing (continuous-integration)

[Gerry Boland (gerboland)]

Is this MR still relevant?

review: Needs Information

[Gerry Boland (gerboland)]

Rejecting as is obsolete

review: Disapprove

Unmerged revisions

147. By Thomas Voß on 2013-12-09

[ Gerry Boland ]
* Unity.Application is not a shared library, but a plugin. (LP:
  #1256014)
* Implement preStart callback added to upstart-app-launch-2. (LP:
  #1243665)
[ Ubuntu daily release ]
* Automatic snapshot from revision 157
[ Gerry Boland ]
* Install ServerStatusListener to be notified of mir server start, pause
  and resume. Use start notification to send SIGSTOP signal to upstart,
  so it knows mir is ready for other clients.
* Bump version number
* InputArea: don't use lambda function as slot, can cause crash on
  shutdown Using lambda function as slot can introduce crash as the
  slot's object deletion is not registered unlike with traditional
  signal/slot connections. As a result, on signal emission, the lambda
  can be called on a deleted object. (LP: #1253685)
* Misc fixes for Mir 0.1.2 support. (LP: #1254986)
[ Alan Griffiths ]
* ApplicationSession is a Mir implementation class that shouldn't be
  used by unity-mir, use shell::session instead.
[ Kevin Gunn ]
* mir server abi and api broke, updating dependency to deb 0.1.1.
* update mir deb build dep to 0.1.2
[ Victor Thompson ]
* Fix mir to not suspend the music-app, or any other app granted a
  lifecycle exception, when switching between apps. (LP: #1241185,
  #1241045)
[ Daniel van Vugt ]
* Force the screen to redraw after turning the display back on (LP:
  #1255045). Also stop the compositor when the screen is off.
  Otherwise it will spin in the background, eating battery. (LP:
  #1255045)
[ Ubuntu daily release ]
* Automatic snapshot from revision 154
* Cherry-pick upstream patch to avoid Unity8 crashing on stop
  (LP: #1253685)
[ Gerry Boland ]
* On MirSurface destruction, any InputAreas on that surface will be
  notified and remove their links to that surface. Fixes crash
  bug:1243444. (LP: #1243444)
[ Alan Griffiths ]
* Remove dependency on mir::shell::SessionManager.
* Remove dependency on mir::surfaces::SurfaceController.
* Remove dependency on msh::OrganisingSurfaceFactory.
* Avoid relying on an explicit Mir typename that changes in a
  refactoring coming soon.
* Do all the hacky surface creation customization in
  SurfaceFactory::create_surface() - and don't mention SurfaceBuilder
  at all. (That will allow Mir to get rid of that interface.).
[ Albert Astals ]
* Don't include the QtQml megaheader Include the one we really need.
[ Ubuntu daily release ]
* Automatic snapshot from revision 144

146. By Thomas Voß on 2013-12-05

Mergeing.

145. By Thomas Voß on 2013-12-05

Adjust to namespace core.

144. By Thomas Voß on 2013-11-12

Refactor sigstop'ing and sigcont'ing of application process groups to rely on process-cpp.

=== modified file 'src/modules/Unity/Application/taskcontroller.cpp'
--- src/modules/Unity/Application/taskcontroller.cpp	2013-12-09 16:39:29 +0000
+++ src/modules/Unity/Application/taskcontroller.cpp	2013-12-09 16:39:29 +0000
@@ -45,14 +45,12 @@
 
 namespace
 {
-void ensureProcessIsUnlikelyToBeKilled(pid_t pid)
+void ensureProcessIsUnlikelyToBeKilled(core::posix::Process process)
 {
     // By system default, we set the oom_score_adj of Unity8 to -10 (via lightdm).
     // As we want to avoid that any app's oom_score_adj is <= Unity8's oom_score_adj,
     // we choose a default increase of +1.
     static const int default_increase = 1;
-    
-    core::posix::Process process(pid);
 
     try
     {
@@ -84,14 +82,12 @@
     }
 }
 
-void ensureProcessIsLikelyToBeKilled(pid_t pid)
+void ensureProcessIsLikelyToBeKilled(core::posix::Process process)
 {
     // We avoid extremal values for oom_score_adj. For that, we
     // set it to 80% of the total available range.
     static const float defaultPercentage = 0.8;
 
-    core::posix::Process process(pid);
-
     try
     {
         plpp::OomScoreAdj shellScore;
@@ -150,7 +146,15 @@
     focusCallback = [](const gchar * appId, gpointer userData) {
         Q_UNUSED(userData)
         pid_t pid = upstart_app_launch_get_primary_pid(appId);
-        ensureProcessIsUnlikelyToBeKilled(pid);
+
+        try
+        {
+            ensureProcessIsUnlikelyToBeKilled(core::posix::Process{pid});
+        } catch(const std::runtime_error&)
+        {
+            LOG("TaskController::startCallback: pid queried from upstart app launch is invalid.");
+        }
+
         Q_EMIT TaskController::singleton()->requestFocus(QString(appId));
     };
 
@@ -232,17 +236,32 @@
     DLOG("TaskController::suspend (this=%p, application=%p)", this, qPrintable(appId));
     pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
 
-    ensureProcessIsLikelyToBeKilled(pid);
-
-    if (pid) {
-        // We do assume that the app was launched by upstart and with that,
-        // in its own process group. For that, we interpret the pid as pgid and
-        // sigstop the complete process group on suspend.
-        kill(-pid, SIGSTOP);
-        return true;
-    } else {
-        return false;
-    }
+    core::posix::Process process = core::posix::Process::invalid();
+    try
+    {
+        process = core::posix::Process{pid};
+    } catch(const std::runtime_error&)
+    {
+        // If the pid reported by upstart app launch is invalid, we error out here.
+        LOG("TaskController::suspend: pid queried from upstart app launch is invalid.");
+        return false;
+    }
+
+    ensureProcessIsLikelyToBeKilled(process);
+
+    // We do assume that the app was launched by upstart and with that,
+    // in its own process group.
+    try
+    {
+        auto pg = process.process_group_or_throw();
+        pg.send_signal_or_throw(core::posix::Signal::sig_stop);
+    } catch(const std::system_error& e)
+    {
+        LOG("TaskController::suspend: Exception sig-stop'ing process group (%s)", e.what());
+        return false;
+    }
+
+    return true;
 }
 
 bool TaskController::resume(const QString& appId)
@@ -250,15 +269,30 @@
     DLOG("TaskController::resume (this=%p, application=%p)", this, qPrintable(appId));
     pid_t pid = upstart_app_launch_get_primary_pid(appId.toLatin1().constData());
 
-    ensureProcessIsUnlikelyToBeKilled(pid);
-
-    if (pid) {
-        // We do assume that the app was launched by upstart and with that,
-        // in its own process group. For that, we interpret the pid as pgid and
-        // sigcont the complete process group on resume.
-        kill(-pid, SIGCONT);
-        return true;
-    } else {
-        return false;
-    }
+    core::posix::Process process = core::posix::Process::invalid();
+    try
+    {
+        process = core::posix::Process{pid};
+    } catch(const std::runtime_error&)
+    {
+        // If the pid reported by upstart app launch is invalid, we error out here.
+        LOG("TaskController::resume: pid queried from upstart app launch is invalid.");
+        return false;
+    }
+
+    ensureProcessIsUnlikelyToBeKilled(process);
+
+    // We do assume that the app was launched by upstart and with that,
+    // in its own process group.
+    try
+    {
+        auto pg = process.process_group_or_throw();
+        pg.send_signal_or_throw(core::posix::Signal::sig_cont);
+    } catch(const std::system_error& e)
+    {
+        LOG("TaskController::resume: Exception sig-continue'ing process group (%s)", e.what());
+        return false;
+    }
+
+    return true;
 }