Page MenuHomePhabricator

D16505.diff
No OneTemporary

D16505.diff

diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php
--- a/src/future/exec/ExecFuture.php
+++ b/src/future/exec/ExecFuture.php
@@ -24,7 +24,6 @@
private $pipes = array();
private $proc = null;
private $start = null;
- private $timeout = null;
private $procStatus = null;
private $stdout = null;
@@ -47,6 +46,10 @@
private $windowsStdoutTempFile = null;
private $windowsStderrTempFile = null;
+ private $terminateTimeout;
+ private $didTerminate;
+ private $killTimeout;
+
private static $descriptorSpec = array(
0 => array('pipe', 'r'), // stdin
1 => array('pipe', 'w'), // stdout
@@ -326,15 +329,20 @@
/**
* Set a hard limit on execution time. If the command runs longer, it will
- * be killed and the future will resolve with an error code. You can test
+ * be terminated and the future will resolve with an error code. You can test
* if a future was killed by a timeout with @{method:getWasKilledByTimeout}.
*
- * @param int Maximum number of seconds this command may execute for.
+ * The subprocess will be sent a `TERM` signal, and then a `KILL` signal a
+ * short while later if it fails to exit.
+ *
+ * @param int Maximum number of seconds this command may execute for before
+ * it is signaled.
* @return this
* @task config
*/
public function setTimeout($seconds) {
- $this->timeout = $seconds;
+ $this->terminateTimeout = $seconds;
+ $this->killTimeout = $seconds + min($seconds, 60);
return $this;
}
@@ -417,13 +425,9 @@
*/
public function resolveKill() {
if (!$this->result) {
- if (defined('SIGKILL')) {
- $signal = SIGKILL;
- } else {
- $signal = 9;
- }
-
+ $signal = 9;
proc_terminate($this->proc, $signal);
+
$this->result = array(
128 + $signal,
$this->stdout,
@@ -787,7 +791,16 @@
}
$elapsed = (microtime(true) - $this->start);
- if ($this->timeout && ($elapsed >= $this->timeout)) {
+
+ if ($this->terminateTimeout && ($elapsed >= $this->terminateTimeout)) {
+ if (!$this->didTerminate) {
+ $this->killedByTimeout = true;
+ $this->sendTerminateSignal();
+ return false;
+ }
+ }
+
+ if ($this->killTimeout && ($elapsed >= $this->killTimeout)) {
$this->killedByTimeout = true;
$this->resolveKill();
return true;
@@ -810,7 +823,10 @@
$status = $this->procGetStatus();
if ($status['running']) {
- $this->resolveKill();
+ $this->sendTerminateSignal();
+ if (!$this->waitForExit(5)) {
+ $this->resolveKill();
+ }
} else {
$this->closeProcess();
}
@@ -901,15 +917,46 @@
public function getDefaultWait() {
$wait = parent::getDefaultWait();
- if ($this->timeout) {
+ $next_timeout = $this->getNextTimeout();
+ if ($next_timeout) {
if (!$this->start) {
$this->start = microtime(true);
}
$elapsed = (microtime(true) - $this->start);
- $wait = max(0, min($this->timeout - $elapsed, $wait));
+ $wait = max(0, min($next_timeout - $elapsed, $wait));
}
return $wait;
}
+ private function getNextTimeout() {
+ if ($this->didTerminate) {
+ return $this->killTimeout;
+ } else {
+ return $this->terminateTimeout;
+ }
+ }
+
+ private function sendTerminateSignal() {
+ $this->didTerminate = true;
+ proc_terminate($this->proc);
+ return $this;
+ }
+
+ private function waitForExit($duration) {
+ $start = microtime(true);
+
+ while (true) {
+ $status = $this->procGetStatus();
+ if (!$status['running']) {
+ return true;
+ }
+
+ $waited = (microtime(true) - $start);
+ if ($waited > $duration) {
+ return false;
+ }
+ }
+ }
+
}
diff --git a/src/future/exec/PhutilSignalRouter.php b/src/future/exec/PhutilSignalRouter.php
--- a/src/future/exec/PhutilSignalRouter.php
+++ b/src/future/exec/PhutilSignalRouter.php
@@ -13,8 +13,12 @@
if (!self::$router) {
$router = new self();
- pcntl_signal(SIGHUP, array($router, 'routeSignal'));
- pcntl_signal(SIGTERM, array($router, 'routeSignal'));
+ // If pcntl_signal() does not exist (particularly, on Windows), just
+ // don't install signal handlers.
+ if (function_exists('pcntl_signal')) {
+ pcntl_signal(SIGHUP, array($router, 'routeSignal'));
+ pcntl_signal(SIGTERM, array($router, 'routeSignal'));
+ }
self::$router = $router;
}

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 7:48 PM (2 d, 1 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7470929
Default Alt Text
D16505.diff (4 KB)

Event Timeline