Linux Chromoting: Explicitly kill all child processes on termination.

Linux Chromoting: Explicitly kill all child processes on termination.

Previously, the script killed only the Xvfb process, relying on the
broken X connection to terminate other processes. This CL ensures that
all managed processes are explicitly killed.

BUG=388957
NOTRY=true

Committed: https://crrev.com/8d49e6119fba636457fcc3d622c676f655a54e6f
Cr-Commit-Position: refs/heads/master@{#299206}

[Lambros, 2014-10-07 22:56:14]

lambroslambrou@chromium.org changed reviewers:
+ rmsousa@chromium.org

[rmsousa, 2014-10-07 23:01:36]

lgtm

lgtm with (optional) suggestion

https://codereview.chromium.org/635953002/diff/1/remoting/host/linux/linux_me...
File remoting/host/linux/linux_me2me_host.py (right):

https://codereview.chromium.org/635953002/diff/1/remoting/host/linux/linux_me...
remoting/host/linux/linux_me2me_host.py:788: proc.terminate()
Since we're changing this logic, should we also wait for the named processes to
die? This avoids the issue with restart where the start part runs before the old
process dies, and we bail because it's already running (or possible issues when
the new process fails to start because the old one hasn't died and relinquished
its resources yet)

[Lambros, 2014-10-08 01:30:54]

Added wait with timeout. PTAL

https://codereview.chromium.org/635953002/diff/1/remoting/host/linux/linux_me...
File remoting/host/linux/linux_me2me_host.py (right):

https://codereview.chromium.org/635953002/diff/1/remoting/host/linux/linux_me...
remoting/host/linux/linux_me2me_host.py:788: proc.terminate()
On 2014/10/07 23:01:36, rmsousa wrote:
> Since we're changing this logic, should we also wait for the named processes
to
> die? This avoids the issue with restart where the start part runs before the
old
> process dies, and we bail because it's already running (or possible issues
when
> the new process fails to start because the old one hasn't died and
relinquished
> its resources yet)

Done.

[Lambros, 2014-10-10 00:42:19]

lambroslambrou@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[rmsousa, 2014-10-10 00:50:27]

lgtm

https://codereview.chromium.org/635953002/diff/40001/remoting/host/linux/linu...
File remoting/host/linux/linux_me2me_host.py (right):

https://codereview.chromium.org/635953002/diff/40001/remoting/host/linux/linu...
remoting/host/linux/linux_me2me_host.py:794: psutil_proc.wait(timeout=10)
This will serialize the process terminations on this wait. Could we send SIGTERM
to the 3 first, then psutil.wait_procs on all 3 at once, then send sigkill to
the ones that are left?

[Lambros, 2014-10-10 01:12:15]

https://codereview.chromium.org/635953002/diff/40001/remoting/host/linux/linu...
File remoting/host/linux/linux_me2me_host.py (right):

https://codereview.chromium.org/635953002/diff/40001/remoting/host/linux/linu...
remoting/host/linux/linux_me2me_host.py:794: psutil_proc.wait(timeout=10)
On 2014/10/10 00:50:27, rmsousa wrote:
> This will serialize the process terminations on this wait. Could we send
SIGTERM
> to the 3 first, then psutil.wait_procs on all 3 at once, then send sigkill to
> the ones that are left?

Nice idea, but probably too complicated to be worthwhile. Most of the time, this
should be quick anyway.

[Lambros, 2014-10-10 22:44:04]

The CQ bit was checked by lambroslambrou@chromium.org

[commit-bot: I haz the power, 2014-10-10 22:44:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/635953002/40001

[commit-bot: I haz the power, 2014-10-10 22:45:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-10 22:46:30]

Patchset 3 (id:??) landed as
https://crrev.com/8d49e6119fba636457fcc3d622c676f655a54e6f
Cr-Commit-Position: refs/heads/master@{#299206}