Page MenuHomePhabricator

When stopping subprocesses, send SIGTERM first, then SIGKILL if that doesn't work
ClosedPublic

Authored by epriestley on Sep 7 2016, 1:18 AM.
Tags
None
Referenced Files
F14078227: D16505.diff
Fri, Nov 22, 2:53 AM
Unknown Object (File)
Thu, Nov 21, 12:55 AM
Unknown Object (File)
Sun, Nov 17, 11:00 PM
Unknown Object (File)
Thu, Nov 14, 6:41 AM
Unknown Object (File)
Sun, Nov 10, 1:49 PM
Unknown Object (File)
Sun, Nov 10, 9:40 AM
Unknown Object (File)
Wed, Nov 6, 9:08 AM
Unknown Object (File)
Wed, Oct 30, 9:22 PM
Subscribers
None

Details

Summary

Fixes T11592. This improves our ability to clean up subprocess trees:

  • When killing a subprocess, send it SIGTERM first. If that doesn't work, wait a little while and send it SIGKILL.
  • When exiting via __destruct() (for example, in response to recieving SIGTERM ourselves) try to SIGTERM children. We wait a shorter period of time for them to exit in this case (5 seconds).
Test Plan

Used several scripts that run one another:

test.php
<?php

require_once '../libphutil/scripts/__init_script__.php';

echo getmypid()."\n";

$future = new ExecFuture('php -f test2.php');
$future->setTimeout(1);
var_dump($future->resolve());
test2.php
<?php

require_once '../libphutil/scripts/__init_script__.php';

echo getmypid()."\n";

$future = new ExecFuture('sleep 240');
$future->resolve();

...and added this snippet to SignalRouter:

$status = getmypid().' got '.$signo."\n";
Filesystem::appendFile('/tmp/signal.log', $status);
echo $status;

Then I ran test.php and signaled it. I saw it TERM the subprocess, which TERM'd the sleep and everything exited properly.

I also ran resist-death.php from libphutil (which ignores signals) with setTimeout() and saw it get TERM'd and then KILL'd a short time later.

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to When stopping subprocesses, send SIGTERM first, then SIGKILL if that doesn't work.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.Sep 7 2016, 4:25 AM
epriestley edited edge metadata.
  • Only install pcntl_signal() conditionally (i.e., not on Windows). Fixes T11603.
  • Use proc_terminate() to send TERM instead of posix_kill(), also for Windows.
This revision was automatically updated to reflect the committed changes.