From 7631cbc6d0323e2a5feeda76c52ecfb53c0be723 Mon Sep 17 00:00:00 2001
From: David Alger <davidmalger@gmail.com>
Date: Thu, 2 May 2019 11:45:27 -0500
Subject: [PATCH] Implement proper error checking around pcntl_waitpid calls

Related to #22563 and #21852
---
 app/code/Magento/Deploy/Process/Queue.php | 47 +++++++++++++++++------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php
index f134dc3246ae5..a80268255aa0d 100644
--- a/app/code/Magento/Deploy/Process/Queue.php
+++ b/app/code/Magento/Deploy/Process/Queue.php
@@ -10,9 +10,9 @@
 use Magento\Deploy\Package\Package;
 use Magento\Deploy\Service\DeployPackage;
 use Magento\Framework\App\ResourceConnection;
-use Psr\Log\LoggerInterface;
 use Magento\Framework\App\State as AppState;
 use Magento\Framework\Locale\ResolverInterface as LocaleResolver;
+use Psr\Log\LoggerInterface;
 
 /**
  * Deployment Queue
@@ -165,6 +165,7 @@ public function process()
         $packages = $this->packages;
         while (count($packages) && $this->checkTimeout()) {
             foreach ($packages as $name => $packageJob) {
+                // Unsets each member of $packages array (passed by reference) as each is executed
                 $this->assertAndExecute($name, $packages, $packageJob);
             }
             $this->logger->info('.');
@@ -224,12 +225,8 @@ private function assertAndExecute($name, array & $packages, array $packageJob)
      * @param bool $dependenciesNotFinished
      * @return void
      */
-    private function executePackage(
-        Package $package,
-        string $name,
-        array &$packages,
-        bool $dependenciesNotFinished
-    ) {
+    private function executePackage(Package $package, string $name, array &$packages, bool $dependenciesNotFinished)
+    {
         if (!$dependenciesNotFinished
             && !$this->isDeployed($package)
             && ($this->maxProcesses < 2 || (count($this->inProgress) < $this->maxProcesses))
@@ -339,13 +336,29 @@ private function isDeployed(Package $package)
         if ($this->isCanBeParalleled()) {
             if ($package->getState() === null) {
                 // phpcs:ignore Magento2.Functions.DiscouragedFunction
-                $pid = pcntl_waitpid($this->getPid($package), $status, WNOHANG);
-                if ($pid === $this->getPid($package)) {
+                $result = pcntl_waitpid($pid, $status, WNOHANG);
+                if ($result === $pid) {
                     $package->setState(Package::STATE_COMPLETED);
+                    $exitStatus = pcntl_wexitstatus($status);
+
+                    $this->logger->info(
+                        "Exited: " . $package->getPath() . "(status: $exitStatus)",
+                        [
+                            'process' => $package->getPath(),
+                            'status' => $exitStatus,
+                        ]
+                    );
 
                     unset($this->inProgress[$package->getPath()]);
                     // phpcs:ignore Magento2.Functions.DiscouragedFunction
                     return pcntl_wexitstatus($status) === 0;
+                } elseif ($result === -1) {
+                    $errno = pcntl_errno();
+                    $strerror = pcntl_strerror($errno);
+
+                    throw new \RuntimeException(
+                        "Error encountered checking child process status (PID: $pid): $strerror (errno: $errno)"
+                    );
                 }
                 return false;
             }
@@ -385,10 +398,22 @@ private function checkTimeout()
     public function __destruct()
     {
         foreach ($this->inProgress as $package) {
+            $pid = $this->getPid($package);
+            $this->logger->info(
+                "Reaping child process: {$package->getPath()} (PID: $pid)",
+                [
+                    'process' => $package->getPath(),
+                    'pid' => $pid,
+                ]
+            );
+
             // phpcs:ignore Magento2.Functions.DiscouragedFunction
-            if (pcntl_waitpid($this->getPid($package), $status) === -1) {
+            if (pcntl_waitpid($pid, $status) === -1) {
+                $errno = pcntl_errno();
+                $strerror = pcntl_strerror($errno);
+
                 throw new \RuntimeException(
-                    'Error while waiting for package deployed: ' . $this->getPid($package) . '; Status: ' . $status
+                    "Error encountered waiting for child process (PID: $pid): $strerror (errno: $errno)"
                 );
             }
         }