Differential D20616 Diff 49183 src/filesystem/PhutilProcessQuery.php

Changeset View

Standalone View

src/filesystem/PhutilProcessQuery.php

Show All 30 Lines	public function execute() {
$processes = phutil_split_lines($processes, false);		$processes = phutil_split_lines($processes, false);

$refs = array();		$refs = array();
foreach ($processes as $process) {		foreach ($processes as $process) {
$parts = preg_split('/\s+/', trim($process), 2);		$parts = preg_split('/\s+/', trim($process), 2);
list($pid, $command) = $parts;		list($pid, $command) = $parts;

$ref = id(new PhutilProcessRef())		$ref = id(new PhutilProcessRef())
->setPID((int)$pid)		->setPID((int)$pid);
->setCommand($command);
		$argv = $this->getArgv($pid, $command);
		$ref->setArgv($argv);

// If this process is a "phd-daemon" process, mark it as a daemon		// If this process is a "phd-daemon" process, mark it as a daemon
// overseer.		// overseer.
		amckinleyUnsubmitted Not Done Inline Actions Comment out of date. amckinley: Comment out of date.
$is_overseer = (bool)preg_match('/\bphd-daemon\b/', $command);		$is_overseer = $ref->getIsOverseer();
$ref->setIsOverseer($is_overseer);

// If this is an overseer and the command has a "-l" ("Label") argument,		// If this is an overseer and the command has a "-l" ("Label") argument,
// the argument contains the "PHABRICATOR_INSTANCE" value for the daemon.		// the argument contains the "PHABRICATOR_INSTANCE" value for the daemon.
// Parse it out and annotate the process.		// Parse it out and annotate the process.
$instance = null;		$instance = null;
if ($ref->getIsOverseer()) {		if ($ref->getIsOverseer()) {
$matches = null;		$matches = null;
if (preg_match('/-l (\S+)/', $command, $matches)) {		if (preg_match('/-l (\S+)/', $command, $matches)) {
Show All 21 Lines	if ($this->instances) {
unset($refs[$key]);		unset($refs[$key]);
}		}
}		}
}		}

return array_values($refs);		return array_values($refs);
}		}

		private function getArgv($pid, $command) {

		// In the output of "ps", arguments in process titles are not escaped, so
		// we can not distinguish between the processes created by running these
		// commands by looking only at the output of "ps":
		amckinleyUnsubmitted Not Done Inline Actions Part of me keeps wanting to say "PID tracking is what you're supposed to do here for exactly these reasons", but since we're only here because PID tracking didn't work very well, I guess there's nothing wrong with trying something else. I was brainstorming/googling other solutions for this, and one (possibly bad) idea I had was starting the daemons with a magic `ENV` variable, like `SECRET_PHAB_COOKIE='🍩'` or something. That would give us the benefit of not relying on regex/string parsing, and possibly insulating us a little more from weird shell gotchas like the above. On the third hand, the worst damage this could do if we got it really wrong would be either failing to start some daemons, or killing an arbitrary process that happened to look sufficiently like a daemon, both of which we could probably figure out and recover from pretty easily. amckinley: Part of me keeps wanting to say "PID tracking is what you're supposed to do here for exactly…
		epriestleyAuthorUnsubmitted Done Inline Actions I think the ENV thing is a reasonable approach if we run into some concrete/hard problem with the args thing. My concerns are: In the most-general case, same problem as args: I don't think we can unambiguously parse ENV out of `ps` since it looks like spaces are unescaped, and I'm not sure there's a fully-correct way to read it on macOS where we don't have `/proc`. That said, we can control ENV better than the process title. ENV propagates to subprocesses, so we need some logic like "process has the secret ENV thing set AND is named phd-daemon" or "...and has no living (?) parents" and/or try really hard to stop the magic from getting into the subprocess environments. Not impossible, but adds more complexity, I think. epriestley: I think the ENV thing is a reasonable approach if we run into some concrete/hard problem with…
		//
		// echo 'a b'
		// echo a b
		//
		// Both commands will have the same process title in the output of "ps".

		// This means we may split the command incorrectly in the general case,
		// and this misparsing may be important if the process binary resides in
		// a directory with spaces in its path and we're trying to identify which
		// binary a process is running.
		amckinleyUnsubmitted Not Done Inline Actions See above >.< amckinley: See above >.<

		// On Ubuntu, and likely most other Linux systems, we can get a raw
		// command line from "/proc" with arguments delimited by "\0".

		// On macOS, there's no "/proc" and we don't currently have a robust way
		// to split the process command in a way that parses spaces properly, so
		// fall back to a best effort based on the output of "ps". This is almost
		// always correct, since it is uncommon to put binaries under paths with
		// spaces in them.

		$proc_cmdline = sprintf('/proc/%d/cmdline', $pid);
		try {
		$argv = Filesystem::readFile($proc_cmdline);
		$argv = explode("\0", $argv);

		// The output itself is terminated with "\0", so remove the final empty
		// argument.
		if (last($argv) === '') {
		array_pop($argv);
		}

		return $argv;
		} catch (Exception $ex) {
		// If we fail to read "/proc", fall through to less reliable methods.
		}

		// If we haven't found a better source, just split the "ps" output on
		// spaces.
		return preg_split('/\s+/', $command);
		}
}		}