Ubuntu Application Launcher

Merge lp://staging/~ted/ubuntu-app-launch/process-group-kill into lp://staging/ubuntu-app-launch/14.04

  1. process-group-kill
  2. Merge into trunk.14.04
Proposed by Ted Gould
Status: Merged
Approved by: Oliver Grawert
Approved revision: 148
Merged at revision: 143
Proposed branch: lp://staging/~ted/ubuntu-app-launch/process-group-kill
Merge into: lp://staging/ubuntu-app-launch/14.04
Diff against target: 26 lines (+10/-2)
1 file modified
upstart-jobs/application-click.conf.in (+10/-2)
To merge this branch: bzr merge lp://staging/~ted/ubuntu-app-launch/process-group-kill
Reviewer Review Type Date Requested Status
Oliver Grawert Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+215475@code.staging.launchpad.net

Commit message

Kill all jobs in process group on exit

Description of the change

After starting record the PID so that we can ensure the PID's process group gets destroyed using pkill after the job stops. A big stick, but insurance.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:147
http://jenkins.qa.ubuntu.com/job/upstart-app-launch-ci/253/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-amd64-ci/112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-armhf-ci/112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-i386-ci/113

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/upstart-app-launch-ci/253/rebuild

review: Approve (continuous-integration)
lp://staging/~ted/ubuntu-app-launch/process-group-kill updated
148. By Ted Gould

Forgot to move over the kill

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:148
http://jenkins.qa.ubuntu.com/job/upstart-app-launch-ci/254/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-amd64-ci/113
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-armhf-ci/113
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-i386-ci/114

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/upstart-app-launch-ci/254/rebuild

review: Approve (continuous-integration)
Revision history for this message
Oliver Grawert (ogra) wrote :

tested, works around the issues in bug 1303676

review: Approve
Revision history for this message
Tyler Hicks (tyhicks) wrote :

While this should work for the purposes of bug 1303676, lets be sure we don't depend, from a security standpoint, on this change to kill all processes spawned by an application. An application could create another process group and move all of its processes to that group and this change would not kill those processes.

I don't think the intent of this change is to fully clean up, with 100% assurance, after an application. As long as that's the case, it looks good to me! :)

Revision history for this message
Oliver Grawert (ogra) wrote :

This change is solely to make the phone image releaseable, we shoulld definitely go on fixing the issue properly.

Revision history for this message
Ted Gould (ted) wrote :

On Fri, 2014-04-11 at 23:05 +0000, Tyler Hicks wrote:

> While this should work for the purposes of bug 1303676, lets be sure we don't depend, from a security standpoint, on this change to kill all processes spawned by an application. An application could create another process group and move all of its processes to that group and this change would not kill those processes.
>
> I don't think the intent of this change is to fully clean up, with 100% assurance, after an application. As long as that's the case, it looks good to me! :)

Yes, that isn't the intent. The question would be whether we pursue a
cgroups based solution for Upstart or wait to implement it on systemd.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches