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

Proposed by Ted Gould
Status: Merged
Approved by: Oliver Grawert
Approved revision: 148
Merged at revision: 143
Proposed branch: lp:~ted/ubuntu-app-launch/process-group-kill
Merge into: lp: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:~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.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 :
review: Approve (continuous-integration)
148. By Ted Gould

Forgot to move over the kill

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
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
1=== modified file 'upstart-jobs/application-click.conf.in'
2--- upstart-jobs/application-click.conf.in 2014-02-13 22:43:55 +0000
3+++ upstart-jobs/application-click.conf.in 2014-04-11 18:33:36 +0000
4@@ -12,6 +12,7 @@
5 env APP_DIR
6 # For Surface Flinger
7 env APP_DESKTOP_FILE
8+env APP_PROCESS_GROUP
9
10 env UPSTART_APP_LAUNCH_ARCH="@upstart_app_launch_arch@"
11 export UPSTART_APP_LAUNCH_ARCH
12@@ -29,5 +30,12 @@
13 # Remember, this is confined
14 exec @pkglibexecdir@/exec-line-exec
15
16-post-start exec @pkglibexecdir@/zg-report-app open
17-post-stop exec @pkglibexecdir@/zg-report-app close
18+post-start script
19+ initctl set-env APP_PROCESS_GROUP=`upstart-app-pid ${APP_ID}`
20+ @pkglibexecdir@/zg-report-app open
21+end script
22+
23+post-stop script
24+ pkill -KILL -g "${APP_PROCESS_GROUP}" || true
25+ @pkglibexecdir@/zg-report-app close
26+end script

Subscribers

People subscribed via source and target branches