Differential D15675 Diff 37877 src/future/exec/ExecFuture.php

Changeset View

Standalone View

src/future/exec/ExecFuture.php

Show First 20 Lines • Show All 383 Lines • ▼ Show 20 Lines	/* -( Configuring Execution )---------------------------------------------- */
* be killed and the future will resolve with an error code. You can test		* be killed and the future will resolve with an error code. You can test
* if a future was killed by a timeout with @{method:getWasKilledByTimeout}.		* if a future was killed by a timeout with @{method:getWasKilledByTimeout}.
*		*
* @param int Maximum number of seconds this command may execute for.		* @param int Maximum number of seconds this command may execute for.
* @return this		* @return this
* @task config		* @task config
*/		*/
public function setTimeout($seconds) {		public function setTimeout($seconds) {
		// Timeout requires non-blocking streams
		$this->setUseWindowsFileStreams(TRUE);
		BYKAuthorUnsubmitted Not Done Inline Actions This fix should probably be in its own patch but I needed it to get my tests passing. BYK: This fix should probably be in its own patch but I needed it to get my tests passing.
		hach-queUnsubmitted Not Done Inline Actions We can probably enable these by default now (i.e. turn on Windows file streams by default instead of explicitly turning it on here). We've been using them for ages and they work fine. hach-que: We can probably enable these by default now (i.e. turn on Windows file streams by default…
		BYKAuthorUnsubmitted Not Done Inline Actions Not turning them on unless necessary avoid temporary file creation etc. so I'm unsure. That said I'm okay if you think that's a good idea. BYK: Not turning them on unless necessary avoid temporary file creation etc. so I'm unsure. That…
$this->timeout = $seconds;		$this->timeout = $seconds;
return $this;		return $this;
}		}


/* -( Resolving Execution )------------------------------------------------ */		/* -( Resolving Execution )------------------------------------------------ */


▲ Show 20 Lines • Show All 73 Lines • ▼ Show 20 Lines	/* -( Resolving Execution )------------------------------------------------ */
public function resolveKill() {		public function resolveKill() {
if (!$this->result) {		if (!$this->result) {
if (defined('SIGKILL')) {		if (defined('SIGKILL')) {
$signal = SIGKILL;		$signal = SIGKILL;
} else {		} else {
$signal = 9;		$signal = 9;
}		}

		if (phutil_is_windows()) {
		// NOTE: See http://stackoverflow.com/a/28045959/90297
		$status = proc_get_status($this->proc);
		exec('taskkill /F /T /PID '.$status['pid']);
		} else {
proc_terminate($this->proc, $signal);		proc_terminate($this->proc, $signal);
		}

$this->result = array(		$this->result = array(
		BYKAuthorUnsubmitted Not Done Inline Actions Same as my previous inline comment. This is a separate issue which I needed to fix to get tests running properly on Windows. BYK: Same as my previous inline comment. This is a separate issue which I needed to fix to get tests…
		BYKAuthorUnsubmitted Not Done Inline Actions Btw. context on this change: http://stackoverflow.com/a/28045959/90297 BYK: Btw. context on this change: http://stackoverflow.com/a/28045959/90297
128 + $signal,		128 + $signal,
$this->stdout,		$this->stdout,
$this->stderr,		$this->stderr,
);		);
$this->closeProcess();		$this->closeProcess();
}		}

return $this->result;		return $this->result;
▲ Show 20 Lines • Show All 151 Lines • ▼ Show 20 Lines	if (!$this->pipes) {

$unmasked_command = $this->command;		$unmasked_command = $this->command;
if ($unmasked_command instanceof PhutilCommandString) {		if ($unmasked_command instanceof PhutilCommandString) {
$unmasked_command = $unmasked_command->getUnmaskedString();		$unmasked_command = $unmasked_command->getUnmaskedString();
}		}

$pipes = array();		$pipes = array();

if (phutil_is_windows()) {
// See T4395. proc_open under Windows uses "cmd /C [cmd]", which will
// strip the first and last quote when there aren't exactly two quotes
// (and some other conditions as well). This results in a command that
// looks like `command" "path to my file" "something something` which is
// clearly wrong. By surrounding the command string with quotes we can
// be sure this process is harmless.
if (strpos($unmasked_command, '"') !== false) {
$unmasked_command = '"'.$unmasked_command.'"';
}
}


// NOTE: Convert all the environmental variables we're going to pass		// NOTE: Convert all the environmental variables we're going to pass
// into strings before we install PhutilErrorTrap. If something in here		// into strings before we install PhutilErrorTrap. If something in here
// is really an object which is going to throw when we try to turn it		// is really an object which is going to throw when we try to turn it
// into a string, we want the exception to escape here -- not after we		// into a string, we want the exception to escape here -- not after we
// start trapping errors.		// start trapping errors.
$env = $this->env;		$env = $this->env;
if ($env !== null) {		if ($env !== null) {
foreach ($env as $key => $value) {		foreach ($env as $key => $value) {
Show All 34 Lines	if (!$this->pipes) {
}		}

$proc = @proc_open(		$proc = @proc_open(
$unmasked_command,		$unmasked_command,
$spec,		$spec,
$pipes,		$pipes,
$cwd,		$cwd,
$env);		$env);

		hach-queUnsubmitted Not Done Inline Actions This will break anything that doesn't specify an absolute path to the binary program, and will break any invocations that make use of shell features such as `&&`. This is not the correct fix. hach-que: This will break anything that doesn't specify an absolute path to the binary program, and will…
		BYKAuthorUnsubmitted Not Done Inline Actions It actually didn't break things that are not specifying an absolute path (see my tests below using just `php`) that said you have a valid point with `&&`. I just don't want to rely on `CMD.exe`'s horribly broken stuff but looks like the solution you want is to simply throw an exception when it sees a new line. Technically `LF` should work where as all `CR`s are removed automatically but I can't even get `LF` working for some reason. BYK: It actually didn't break things that are not specifying an absolute path (see my tests below…
if ($this->useWindowsFileStreams) {		if ($this->useWindowsFileStreams) {
fclose($spec[1]);		fclose($spec[1]);
fclose($spec[2]);		fclose($spec[2]);
$pipes = array(		$pipes = array(
0 => head($pipes), // stdin		0 => head($pipes), // stdin
1 => fopen($this->windowsStdoutTempFile, 'rb'), // stdout		1 => fopen($this->windowsStdoutTempFile, 'rb'), // stdout
2 => fopen($this->windowsStderrTempFile, 'rb'), // stderr		2 => fopen($this->windowsStderrTempFile, 'rb'), // stderr
);		);
▲ Show 20 Lines • Show All 102 Lines • ▼ Show 20 Lines	if (!$status['running']) {
// consider the subprocess to have exited until we've read everything.		// consider the subprocess to have exited until we've read everything.
// See T9724 for context.		// See T9724 for context.
if (feof($stdout) && feof($stderr)) {		if (feof($stdout) && feof($stderr)) {
$is_done = true;		$is_done = true;
}		}
}		}

if ($is_done) {		if ($is_done) {
if ($this->useWindowsFileStreams) {
fclose($stdout);
fclose($stderr);
}
BYKAuthorUnsubmitted Not Done Inline Actions `closeProcess` already closes all pipes so this is redundant. BYK: `closeProcess` already closes all pipes so this is redundant.

$this->result = array(		$this->result = array(
$status['exitcode'],		$status['exitcode'],
$this->stdout,		$this->stdout,
$this->stderr,		$this->stderr,
);		);
$this->closeProcess();		$this->closeProcess();
return true;		return true;
}		}
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines