Phd status fails to get running daemons when the php binary is not called php

Hi.

I’ve noticed that in order to find the PID of the running daemons Phabricator looks over the list of processes for one of these matches:

  • /path/to/phd-daemon
  • /path/to/php /path/to/phd-daemon

This logic is implemented in the libphutil/src/filesystem/PhutilProcessRef.php file:

public function getIsOverseer() {
  if ($this->isOverseer === null) {
    $this->isOverseer = $this->getCommandMatch(
      array(
        array('phd-daemon'),
        array('php', 'phd-daemon'),
      ));
  }

  return $this->isOverseer;
}

private function getCommandMatch(array $patterns) {
  $argv = $this->getArgv();

  foreach ($patterns as $pattern) {
    $pattern = array_values($pattern);
    $is_match = true;
    for ($ii = 0; $ii < count($pattern); $ii++) {
      if (!isset($argv[$ii])) {
        $is_match = false;
        break;
      }

      if (basename($argv[$ii]) !== $pattern[$ii]) {
        $is_match = false;
        break;
      }
    }

    if ($is_match) {
      return true;
    }
  }

  return false;
}

The problem here is that there’s no guarantee that the php binary will be called php. For example, I am a Bitnami developer and we work with a wrapper around the php binary, which in our case is actually called php.bin. To solve this issue, PHP provides a predefined constant, PHP_BINARY, that returns the PHP binary path during script execution.

Therefore, I propose to use the PHP_BINARY constant instead of the hardcoded php binary name. It would be something like this:

public function getIsOverseer() {
  if ($this->isOverseer === null) {
    $this->isOverseer = $this->getCommandMatch(
      array(
        array('phd-daemon'),
        array(basename(PHP_BINARY), 'phd-daemon'),
      ));
  }

  return $this->isOverseer;
}

What do you think?

Reproduction Instructions
Run Phabricator with a php binary not named php and then run phd status. phd won’t be able to detect the running daemon.

Phabricator/Arcanist Version
arcanist feb5f4d42c4fe0001e76428e80d5e88262308802 (22 Jun 2019)
libphutil a4feaf52f4c01ff692e46e53bb731c9d7e5bedfe (28 Jun 2019)

Generally, we won’t make upstream changes exclusively to support a single install or customer.

We may consider this change (or some similar change) if we learn that running with a binary that is not named php is common in the future, and that there’s good technical rationale for this choice, but I’d guess this is not likely.

@epriestley for us (Bitnami) it’s simple to fix this on our side, so, while we prefer to be fully in sync with the upstream projects we package, if you prefer to not solve it until more folks hit the problem, it’s ok to us.

In the future, if you happen to fix this issue, would you mind to update this support ticket so we get notified about it?

Thanks for you help.