From 49b3c7213d6a968f478d479e2063855573acd880 Mon Sep 17 00:00:00 2001 From: David Alger Date: Thu, 2 May 2019 11:39:20 -0500 Subject: [PATCH 1/5] Resolves incorrect boolean return type in getPid Reverts inadvertent change in 7421dfb causing the improper type to be returned as noted on #22563 --- app/code/Magento/Deploy/Process/Queue.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php index fd7aad44e0a5b..f134dc3246ae5 100644 --- a/app/code/Magento/Deploy/Process/Queue.php +++ b/app/code/Magento/Deploy/Process/Queue.php @@ -361,7 +361,7 @@ private function isDeployed(Package $package) */ private function getPid(Package $package) { - return isset($this->processIds[$package->getPath()]) ?? null; + return $this->processIds[$package->getPath()] ?? null; } /** From 7631cbc6d0323e2a5feeda76c52ecfb53c0be723 Mon Sep 17 00:00:00 2001 From: David Alger Date: Thu, 2 May 2019 11:45:27 -0500 Subject: [PATCH 2/5] 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)" ); } } From e1525a80036e89c7289e22530067db522441c555 Mon Sep 17 00:00:00 2001 From: David Alger Date: Thu, 2 May 2019 11:46:31 -0500 Subject: [PATCH 3/5] Fixes two issues; the type error surfaced by use of strict_types in 2.3-develop and issue where SCD hangs ending in RuntimeException on 2.2-develop Resolves #22563 and #21852 --- app/code/Magento/Deploy/Process/Queue.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php index a80268255aa0d..1fb8828844b2e 100644 --- a/app/code/Magento/Deploy/Process/Queue.php +++ b/app/code/Magento/Deploy/Process/Queue.php @@ -335,6 +335,11 @@ private function isDeployed(Package $package) { if ($this->isCanBeParalleled()) { if ($package->getState() === null) { + $pid = $this->getPid($package); + if ($pid === null) { + return false; + } + // phpcs:ignore Magento2.Functions.DiscouragedFunction $result = pcntl_waitpid($pid, $status, WNOHANG); if ($result === $pid) { From 8d0a3d8acf11807773a326f4dc03ca8d9704dc77 Mon Sep 17 00:00:00 2001 From: David Alger Date: Thu, 2 May 2019 11:51:14 -0500 Subject: [PATCH 4/5] Added a comment to explain the return on null pid --- app/code/Magento/Deploy/Process/Queue.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php index 1fb8828844b2e..1b3944a884e4c 100644 --- a/app/code/Magento/Deploy/Process/Queue.php +++ b/app/code/Magento/Deploy/Process/Queue.php @@ -336,6 +336,9 @@ private function isDeployed(Package $package) if ($this->isCanBeParalleled()) { if ($package->getState() === null) { $pid = $this->getPid($package); + + // When $pid comes back as null the child process for this package has not yet started; prevents both + // hanging until timeout expires (which was behaviour in 2.2.x) and the type error from strict_types if ($pid === null) { return false; } From 2be310574b2b8bb4832300af4a50e448ff19da90 Mon Sep 17 00:00:00 2001 From: Nazarn96 Date: Thu, 16 May 2019 11:26:14 +0300 Subject: [PATCH 5/5] magento/magento2#22607 static-test-fix --- app/code/Magento/Deploy/Process/Queue.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php index 1b3944a884e4c..2f2a149239990 100644 --- a/app/code/Magento/Deploy/Process/Queue.php +++ b/app/code/Magento/Deploy/Process/Queue.php @@ -347,6 +347,7 @@ private function isDeployed(Package $package) $result = pcntl_waitpid($pid, $status, WNOHANG); if ($result === $pid) { $package->setState(Package::STATE_COMPLETED); + // phpcs:ignore Magento2.Functions.DiscouragedFunction $exitStatus = pcntl_wexitstatus($status); $this->logger->info( @@ -361,7 +362,9 @@ private function isDeployed(Package $package) // phpcs:ignore Magento2.Functions.DiscouragedFunction return pcntl_wexitstatus($status) === 0; } elseif ($result === -1) { + // phpcs:ignore Magento2.Functions.DiscouragedFunction $errno = pcntl_errno(); + // phpcs:ignore Magento2.Functions.DiscouragedFunction $strerror = pcntl_strerror($errno); throw new \RuntimeException( @@ -401,6 +404,7 @@ private function checkTimeout() * Protect against zombie process * * @throws \RuntimeException + * @SuppressWarnings(PHPMD.UnusedLocalVariable) * @return void */ public function __destruct() @@ -417,7 +421,9 @@ public function __destruct() // phpcs:ignore Magento2.Functions.DiscouragedFunction if (pcntl_waitpid($pid, $status) === -1) { + // phpcs:ignore Magento2.Functions.DiscouragedFunction $errno = pcntl_errno(); + // phpcs:ignore Magento2.Functions.DiscouragedFunction $strerror = pcntl_strerror($errno); throw new \RuntimeException(