Streamline handling of Futures and PIDs in daemons

Summary:
Ref T13555. Currently, the daemon future may resolve into a failure state immediately inside "start()", and not have a valid PID when we read it.

Instead, read PIDs from the current active future in all cases, using "hasPID()" to test for the presence of a valid PID.

Since we don't query the PID immediately, we no longer need to explicitly start the future.

Also fix an issue where the same future could be added to the overseer pool more than once if it threw on "resolve()". In general:

- Before we "resolve()" a future, detach it from the DaemonHandle: we're always done with it.
- Catch exceptions on resolution and treat them the same way as subprocess resolution errors. These aren't common, but are possible in the general case.
- Have DaemonHandle add futures to the future pool directly when they're created.

Test Plan:
- Ran daemons with intentional subprocess creation failures, saw clean recovery.
- Ran daemons with intentional resolution exceptions, saw clean recovery.

Maniphest Tasks: T13555

Differential Revision: https://secure.phabricator.com/D21425

epriestley 5 years ago 78d1b62b 5f053593

+51 -37

3 changed files

expand all

src

infrastructure

daemon

PhutilDaemonHandle.php

PhutilDaemonOverseer.php

PhutilDaemonPool.php

+46 -21

src/infrastructure/daemon/PhutilDaemonHandle.php

··· 16 16 private $restartAt; 17 17 private $busyEpoch; 18 18 19 - private $pid; 20 19 private $daemonID; 21 20 private $deadline; 22 21 private $heartbeat; ··· 104 103 } 105 104 106 105 public function isRunning() { 107 - return (bool)$this->future; 106 + return (bool)$this->getFuture(); 108 107 } 109 108 110 109 public function isHibernating() { ··· 134 133 return (!$this->shouldRestart && !$this->isRunning()); 135 134 } 136 135 137 - public function getFuture() { 138 - return $this->future; 139 - } 140 - 141 136 public function update() { 142 137 if (!$this->isRunning()) { 143 138 if (!$this->shouldRestart) { ··· 152 147 $this->startDaemonProcess(); 153 148 } 154 149 155 - $future = $this->future; 150 + $future = $this->getFuture(); 156 151 157 152 $result = null; 158 - if ($future->isReady()) { 159 - $result = $future->resolve(); 153 + $caught = null; 154 + if ($future->canResolve()) { 155 + $this->future = null; 156 + try { 157 + $result = $future->resolve(); 158 + } catch (Exception $ex) { 159 + $caught = $ex; 160 + } catch (Throwable $ex) { 161 + $caught = $ex; 162 + } 160 163 } 161 164 162 165 list($stdout, $stderr) = $future->read(); ··· 173 176 } 174 177 } 175 178 176 - if ($result !== null) { 177 - list($err) = $result; 179 + if ($result !== null || $caught !== null) { 178 180 179 - if ($err) { 180 - $this->logMessage('FAIL', pht('Process exited with error %s.', $err)); 181 + if ($caught) { 182 + $message = pht( 183 + 'Process failed with exception: %s', 184 + $caught->getMessage()); 185 + $this->logMessage('FAIL', $message); 181 186 } else { 182 - $this->logMessage('DONE', pht('Process exited normally.')); 187 + list($err) = $result; 188 + 189 + if ($err) { 190 + $this->logMessage('FAIL', pht('Process exited with error %s.', $err)); 191 + } else { 192 + $this->logMessage('DONE', pht('Process exited normally.')); 193 + } 183 194 } 184 - 185 - $this->future = null; 186 195 187 196 if ($this->shouldShutdown) { 188 197 $this->restartAt = null; ··· 244 253 return $this->daemonID; 245 254 } 246 255 247 - public function getPID() { 248 - return $this->pid; 256 + private function getFuture() { 257 + return $this->future; 258 + } 259 + 260 + private function getPID() { 261 + $future = $this->getFuture(); 262 + 263 + if (!$future) { 264 + return null; 265 + } 266 + 267 + if (!$future->hasPID()) { 268 + return null; 269 + } 270 + 271 + return $future->getPID(); 249 272 } 250 273 251 274 private function getCaptureBufferSize() { ··· 346 369 $this->stdoutBuffer = ''; 347 370 $this->hibernating = false; 348 371 349 - $this->future = $this->newExecFuture(); 350 - $this->future->start(); 372 + $future = $this->newExecFuture(); 373 + $this->future = $future; 351 374 352 - $this->pid = $this->future->getPID(); 375 + $pool = $this->getDaemonPool(); 376 + $overseer = $pool->getOverseer(); 377 + $overseer->addFutureToPool($future); 353 378 } 354 379 355 380 private function didReadStdout($data) {

+5 -4

src/infrastructure/daemon/PhutilDaemonOverseer.php

··· 181 181 } 182 182 } 183 183 184 - foreach ($pool->getFutures() as $future) { 185 - $future_pool->addFuture($future); 186 - } 187 - 188 184 if ($pool->getDaemons()) { 189 185 $running_pools = true; 190 186 } ··· 208 204 } 209 205 210 206 exit($this->err); 207 + } 208 + 209 + public function addFutureToPool(Future $future) { 210 + $this->getFuturePool()->addFuture($future); 211 + return $this; 211 212 } 212 213 213 214 private function getFuturePool() {

-12

src/infrastructure/daemon/PhutilDaemonPool.php

··· 111 111 return $this->daemons; 112 112 } 113 113 114 - public function getFutures() { 115 - $futures = array(); 116 - foreach ($this->getDaemons() as $daemon) { 117 - $future = $daemon->getFuture(); 118 - if ($future) { 119 - $futures[] = $future; 120 - } 121 - } 122 - 123 - return $futures; 124 - } 125 - 126 114 public function didReceiveSignal($signal, $signo) { 127 115 switch ($signal) { 128 116 case PhutilDaemonOverseer::SIGNAL_GRACEFUL:

Configure Feed

Configure Feed