From 9dd1ec9a36e9e169758bb99698d407916bdec41c Mon Sep 17 00:00:00 2001 From: mucha55 Date: Mon, 13 Sep 2021 11:42:24 -0500 Subject: [PATCH 1/6] KOJO-242 | Process group awareness --- src/ProcessAbstract.php | 13 +++++++++++++ src/ProcessInterface.php | 3 +++ 2 files changed, 16 insertions(+) diff --git a/src/ProcessAbstract.php b/src/ProcessAbstract.php index 6d083f4e..57684e59 100644 --- a/src/ProcessAbstract.php +++ b/src/ProcessAbstract.php @@ -29,6 +29,7 @@ protected function _initialize(): ProcessAbstract $this->_getApmNewRelic()->endTransaction(); $this->_setParentProcessId(posix_getppid()); $this->_setProcessId(posix_getpid()); + $this->_setProcessGroupId(posix_getpgrp()); $this->getProcessPoolLoggerMessageMetadataBuilder()->setProcess($this); if ($this->_hasProcessPool()) { @@ -203,6 +204,18 @@ public function getParentProcessId(): int return $this->_read(self::PROP_PARENT_PROCESS_ID); } + protected function _setProcessGroupId(int $processGroupId): ProcessAbstract + { + $this->_create(self::PROP_PROCESS_GROUP_ID, $processGroupId); + + return $this; + } + + public function getProcessGroupId(): int + { + return $this->_read(self::PROP_PROCESS_GROUP_ID); + } + public function setExitCode(int $exitCode): ProcessInterface { $this->_create(self::PROP_EXIT_CODE, $exitCode); diff --git a/src/ProcessInterface.php b/src/ProcessInterface.php index 599eb8a6..2a76c68c 100644 --- a/src/ProcessInterface.php +++ b/src/ProcessInterface.php @@ -14,6 +14,7 @@ interface ProcessInterface extends HandlerInterface public const PROP_PATH = 'path'; public const PROP_TERMINATION_SIGNAL_NUMBER = 'termination_signal_number'; public const PROP_PROCESS_ID = 'process_id'; + public const PROP_PROCESS_GROUP_ID = 'process_group_id'; public const PROP_TYPE_CODE = 'type_code'; public const PROP_UUID = 'uuid'; public const PROP_UUID_MAXIMUM_INTEGER = 'uuid_maximum_integer'; @@ -29,6 +30,8 @@ public function start(): ProcessInterface; public function getProcessId(): int; + public function getProcessGroupId(): int; + public function setLogger(LoggerInterface $logger); public function setThrottle(int $seconds = 0): ProcessInterface; From d0c204cff224ae89a38325b14ddfc4afd9c0e07c Mon Sep 17 00:00:00 2001 From: mucha55 Date: Mon, 13 Sep 2021 11:43:28 -0500 Subject: [PATCH 2/6] KOJO-243 | Orphaned process detection logic --- src/Process/Pool.php | 76 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/Process/Pool.php b/src/Process/Pool.php index 4cc6273f..97e3782d 100644 --- a/src/Process/Pool.php +++ b/src/Process/Pool.php @@ -51,6 +51,12 @@ protected function _childExitSignal(InformationInterface $information): PoolInte $this->_getLogger()->debug("Child process[$childProcessId] is not in the pool for process[$processId]."); } + if ($information->getExitValue() === SIGKILL) { + $this->_getLogger()->info('SIGKILL detected'); + + $this->inspectProcDirectory(); + } + return $this; } @@ -131,4 +137,74 @@ public function emptyChildProcesses(): PoolInterface return $this; } + + private function inspectProcDirectory() : PoolInterface + { + // get the status file for every process + foreach (glob('/proc/[0-9]*/status') as $procStatusFile) { + $processId = null; + $parentProcessId = null; + $processGroupId = null; + + try { + $procStatusFd = fopen($procStatusFile, 'r'); + + // parse the file for the IDs above + while (($line = fgets($procStatusFd))) { + list($item) = sscanf($line, "Pid:\t%s"); + if ($item !== null) { + $processId = (int)$item; + } + + list($item) = sscanf($line, "PPid:\t%s"); + if ($item !== null) { + $parentProcessId = (int)$item; + } + + list($item) = sscanf($line, "NSpgid:\t%s"); + if ($item !== null) { + $processGroupId = (int)$item; + } + + // if we've extracted all the information we need, + // check if this is a process we need to clean up + if ($processId && $parentProcessId && $processGroupId) { + if ( + // was it orphaned + $parentProcessId === 1 && + // was it spawned from the same ancestor (i.e. the Server) + $processGroupId === $this->getProcess()->getProcessGroupId() && + // guard against false positives + // is not the Root + $processId !== $this->getProcess()->getProcessId() && + // is not init + $processId !== 1 + ) { + $this->_getLogger()->warning( + 'Terminating orphaned process', + [ + 'process_id' => $processId, + 'parent_process_id' => $parentProcessId, + 'process_group_id' => $processGroupId, + // Root information is included in the kojo_metadata + ] + ); + // SIGKILL must be used here because watchdogs won't handle any signals + // any (unexpected) orphan processes that result from this will be + // cleaned up in the next pass + posix_kill($processId, SIGKILL); + } + // move on to the next /proc/ file regardless + break; + } + } + } finally { + if ($procStatusFd !== false) { + fclose($procStatusFd); + } + } + } + + return $this; + } } From 2dc27b667f481dc03ae849e0b32b10f19a874c21 Mon Sep 17 00:00:00 2001 From: mucha55 Date: Tue, 21 Sep 2021 16:35:23 -0500 Subject: [PATCH 3/6] KOJO-243 | Use notice for SIGKILL detection log message --- src/Process/Pool.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Process/Pool.php b/src/Process/Pool.php index 97e3782d..71177f47 100644 --- a/src/Process/Pool.php +++ b/src/Process/Pool.php @@ -52,7 +52,7 @@ protected function _childExitSignal(InformationInterface $information): PoolInte } if ($information->getExitValue() === SIGKILL) { - $this->_getLogger()->info('SIGKILL detected'); + $this->_getLogger()->notice('SIGKILL detected'); $this->inspectProcDirectory(); } From 7f768a74c40d89a1c8af3bd11b9ce5805c20a2bf Mon Sep 17 00:00:00 2001 From: mucha55 Date: Fri, 8 Oct 2021 14:47:12 -0500 Subject: [PATCH 4/6] KOJO-243 | Handle deleted /proc/ files gracefully --- src/Process/Pool.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Process/Pool.php b/src/Process/Pool.php index 71177f47..f1f51a1d 100644 --- a/src/Process/Pool.php +++ b/src/Process/Pool.php @@ -147,7 +147,15 @@ private function inspectProcDirectory() : PoolInterface $processGroupId = null; try { - $procStatusFd = fopen($procStatusFile, 'r'); + // files in /proc/ can be deleted at any time + // suppress PHP's warning (which will become an error) + // we'll check for failure after attempting to open + $procStatusFd = @fopen($procStatusFile, 'r'); + + // this file has been deleted (or we don't have permission) + if ($procStatusFd === false) { + continue; + } // parse the file for the IDs above while (($line = fgets($procStatusFd))) { From a31d82307765c86ad46f75bbf96b2275855d649c Mon Sep 17 00:00:00 2001 From: mucha55 Date: Fri, 8 Oct 2021 15:22:43 -0500 Subject: [PATCH 5/6] KOJO-243 | Guard against anyone but the Root cleaning up orphans --- src/Process/Pool.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Process/Pool.php b/src/Process/Pool.php index f1f51a1d..30be9484 100644 --- a/src/Process/Pool.php +++ b/src/Process/Pool.php @@ -51,7 +51,10 @@ protected function _childExitSignal(InformationInterface $information): PoolInte $this->_getLogger()->debug("Child process[$childProcessId] is not in the pool for process[$processId]."); } - if ($information->getExitValue() === SIGKILL) { + if ( + $information->getExitValue() === SIGKILL && + $this->getProcess() instanceof Root + ) { $this->_getLogger()->notice('SIGKILL detected'); $this->inspectProcDirectory(); From fa8c359321b2eb8bb997bbd8c1e68e85a0da2f91 Mon Sep 17 00:00:00 2001 From: mucha55 Date: Thu, 28 Oct 2021 14:15:58 -0500 Subject: [PATCH 6/6] KOJO-243 | Move proc inspection into process pool strategy --- src/Process/Pool.php | 87 +------------------------- src/Process/Pool/Strategy.php | 80 +++++++++++++++++++++++ src/Process/Pool/Strategy/Server.php | 9 +++ src/Process/Pool/Strategy/Worker.php | 8 +++ src/Process/Pool/StrategyInterface.php | 4 +- 5 files changed, 102 insertions(+), 86 deletions(-) diff --git a/src/Process/Pool.php b/src/Process/Pool.php index 30be9484..2d13c4bb 100644 --- a/src/Process/Pool.php +++ b/src/Process/Pool.php @@ -51,13 +51,8 @@ protected function _childExitSignal(InformationInterface $information): PoolInte $this->_getLogger()->debug("Child process[$childProcessId] is not in the pool for process[$processId]."); } - if ( - $information->getExitValue() === SIGKILL && - $this->getProcess() instanceof Root - ) { - $this->_getLogger()->notice('SIGKILL detected'); - - $this->inspectProcDirectory(); + if ($information->getExitValue() === SIGKILL) { + $this->_getProcessPoolStrategy()->handlePotentiallyStrayProcesses(); } return $this; @@ -140,82 +135,4 @@ public function emptyChildProcesses(): PoolInterface return $this; } - - private function inspectProcDirectory() : PoolInterface - { - // get the status file for every process - foreach (glob('/proc/[0-9]*/status') as $procStatusFile) { - $processId = null; - $parentProcessId = null; - $processGroupId = null; - - try { - // files in /proc/ can be deleted at any time - // suppress PHP's warning (which will become an error) - // we'll check for failure after attempting to open - $procStatusFd = @fopen($procStatusFile, 'r'); - - // this file has been deleted (or we don't have permission) - if ($procStatusFd === false) { - continue; - } - - // parse the file for the IDs above - while (($line = fgets($procStatusFd))) { - list($item) = sscanf($line, "Pid:\t%s"); - if ($item !== null) { - $processId = (int)$item; - } - - list($item) = sscanf($line, "PPid:\t%s"); - if ($item !== null) { - $parentProcessId = (int)$item; - } - - list($item) = sscanf($line, "NSpgid:\t%s"); - if ($item !== null) { - $processGroupId = (int)$item; - } - - // if we've extracted all the information we need, - // check if this is a process we need to clean up - if ($processId && $parentProcessId && $processGroupId) { - if ( - // was it orphaned - $parentProcessId === 1 && - // was it spawned from the same ancestor (i.e. the Server) - $processGroupId === $this->getProcess()->getProcessGroupId() && - // guard against false positives - // is not the Root - $processId !== $this->getProcess()->getProcessId() && - // is not init - $processId !== 1 - ) { - $this->_getLogger()->warning( - 'Terminating orphaned process', - [ - 'process_id' => $processId, - 'parent_process_id' => $parentProcessId, - 'process_group_id' => $processGroupId, - // Root information is included in the kojo_metadata - ] - ); - // SIGKILL must be used here because watchdogs won't handle any signals - // any (unexpected) orphan processes that result from this will be - // cleaned up in the next pass - posix_kill($processId, SIGKILL); - } - // move on to the next /proc/ file regardless - break; - } - } - } finally { - if ($procStatusFd !== false) { - fclose($procStatusFd); - } - } - } - - return $this; - } } diff --git a/src/Process/Pool/Strategy.php b/src/Process/Pool/Strategy.php index ac30df6a..8c67edb1 100644 --- a/src/Process/Pool/Strategy.php +++ b/src/Process/Pool/Strategy.php @@ -213,4 +213,84 @@ protected function _unPauseListenerProcesses(): Strategy return $this; } + + public function handlePotentiallyStrayProcesses() : StrategyInterface + { + $this->_getLogger()->notice('SIGKILL of Worker process detected'); + + // get the status file for every process + foreach (glob('/proc/[0-9]*/status') as $procStatusFile) { + $processId = null; + $parentProcessId = null; + $processGroupId = null; + + try { + // files in /proc/ can be deleted at any time + // suppress PHP's warning (which will become an error) + // we'll check for failure after attempting to open + $procStatusFd = @fopen($procStatusFile, 'r'); + + // this file has been deleted (or we don't have permission) + if ($procStatusFd === false) { + continue; + } + + // parse the file for the IDs above + while (($line = fgets($procStatusFd))) { + [$item] = sscanf($line, "Pid:\t%s"); + if ($item !== null) { + $processId = (int)$item; + } + + [$item] = sscanf($line, "PPid:\t%s"); + if ($item !== null) { + $parentProcessId = (int)$item; + } + + [$item] = sscanf($line, "NSpgid:\t%s"); + if ($item !== null) { + $processGroupId = (int)$item; + } + + // if we've extracted all the information we need, + // check if this is a process we need to clean up + if ($processId && $parentProcessId && $processGroupId) { + if ( + // was it orphaned + $parentProcessId === 1 && + // was it spawned from the same ancestor (i.e. the Server) + $processGroupId === $this->_getProcessPool()->getProcess()->getProcessGroupId() && + // guard against false positives + // is not the Root + $processId !== $this->_getProcessPool()->getProcess()->getProcessId() && + // is not init + $processId !== 1 + ) { + $this->_getLogger()->warning( + 'Terminating orphaned process', + [ + 'process_id' => $processId, + 'parent_process_id' => $parentProcessId, + 'process_group_id' => $processGroupId, + // Root information is included in the kojo_metadata + ] + ); + // SIGKILL must be used here because watchdogs won't handle any signals + // any (unexpected) orphan processes that result from this will be + // cleaned up in the next pass + posix_kill($processId, SIGKILL); + } + // move on to the next /proc/ file regardless + break; + } + } + } finally { + if ($procStatusFd !== false) { + fclose($procStatusFd); + } + } + } + + return $this; + } } diff --git a/src/Process/Pool/Strategy/Server.php b/src/Process/Pool/Strategy/Server.php index 40d4b7a9..4d9eea08 100644 --- a/src/Process/Pool/Strategy/Server.php +++ b/src/Process/Pool/Strategy/Server.php @@ -67,4 +67,13 @@ public function initializePool(): StrategyInterface return $this; } + + public function handlePotentiallyStrayProcesses() : StrategyInterface + { + // since the Root is the only child of the Server, this will only be invoked on SIGKILL of the Root + // that shouldn't be a problem, since all the children of the Root are responsible for terminating + // themselves (as opposed to children of Workers, which need their parents to terminate them) + + return $this; + } } diff --git a/src/Process/Pool/Strategy/Worker.php b/src/Process/Pool/Strategy/Worker.php index 33792968..882b64c2 100644 --- a/src/Process/Pool/Strategy/Worker.php +++ b/src/Process/Pool/Strategy/Worker.php @@ -70,4 +70,12 @@ public function initializePool(): StrategyInterface return $this; } + + public function handlePotentiallyStrayProcesses() : StrategyInterface + { + // Watchdogs are always SIGKILLed because they need to be unable to handle signals like SIGTERM + // so this is normal operation, no action needs to be taken + + return $this; + } } diff --git a/src/Process/Pool/StrategyInterface.php b/src/Process/Pool/StrategyInterface.php index 3bca034e..4bebd110 100644 --- a/src/Process/Pool/StrategyInterface.php +++ b/src/Process/Pool/StrategyInterface.php @@ -47,4 +47,6 @@ public function setFillProcessTypeCode(string $fillProcessTypeCode): StrategyInt public function setMaximumLoadAverage(float $maximumLoadAverage): StrategyInterface; public function getMaximumLoadAverage(): float; -} \ No newline at end of file + + public function handlePotentiallyStrayProcesses() : StrategyInterface; +}