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); | |||||
// If this process is a "phd-daemon" process, mark it as a daemon | $argv = $this->getArgv($pid, $command); | ||||
amckinley: Comment out of date. | |||||
// overseer. | $ref->setArgv($argv); | ||||
$is_overseer = (bool)preg_match('/\bphd-daemon\b/', $command); | |||||
$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": | |||||
Not Done Inline ActionsPart 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… | |||||
Done Inline ActionsI think the ENV thing is a reasonable approach if we run into some concrete/hard problem with the args thing. My concerns are:
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. | |||||
Not Done Inline ActionsSee 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); | |||||
} | |||||
} | } |
Comment out of date.