fix(PidBasedController) check PID maps to running process

This fixes the issue where one of the managed processes (Chromedriver, PHP built-in server or MySQL server) would be killed by the system or user outside of wp-browser control leaving an orphaned PID file pointing at a no-more-running process.
lucatume · Nov 29, 2024 · 9e78108 · 9e78108
1 parent 7163a8e
commit 9e78108
Show file tree

Hide file tree

Showing 7 changed files with 209 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [unreleased] Unreleased
 
+## Fixed
+
+- Check the PID file for the PHP built-in server, MySQL and Chromedriver controllers to make sure the PID maps to an actually running process.
+
 ## [4.3.9] 2024-11-29;
 
 ## Changed

diff --git a/src/Extension/BuiltInServerController.php b/src/Extension/BuiltInServerController.php
@@ -19,7 +19,7 @@ public function start(OutputInterface $output): void
     {
         $pidFile = $this->getPidFile();
 
-        if (is_file($pidFile)) {
+        if ($this->isProcessRunning($pidFile)) {
             $output->writeln('PHP built-in server already running.');
             return;
         }

diff --git a/src/Extension/ChromeDriverController.php b/src/Extension/ChromeDriverController.php
@@ -20,7 +20,7 @@ public function start(OutputInterface $output): void
     {
         $pidFile = $this->getPidFile();
 
-        if (is_file($pidFile)) {
+        if ($this->isProcessRunning($pidFile)) {
             $output->writeln('ChromeDriver already running.');
 
             return;

diff --git a/src/Extension/MysqlServerController.php b/src/Extension/MysqlServerController.php
@@ -20,7 +20,7 @@ public function start(OutputInterface $output): void
     {
         $pidFile = $this->getPidFile();
 
-        if (is_file($pidFile)) {
+        if ($this->isProcessRunning($pidFile)) {
             $output->writeln('MySQL server already running.');
 
             return;

diff --git a/src/Extension/PidBasedController.php b/src/Extension/PidBasedController.php
@@ -3,6 +3,7 @@
 namespace lucatume\WPBrowser\Extension;
 
 use Codeception\Exception\ExtensionException;
+use lucatume\WPBrowser\Exceptions\RuntimeException;
 
 trait PidBasedController
 {
@@ -49,4 +50,51 @@ protected function removePidFile(string $pidFile): void
             );
         }
     }
+
+    /**
+     * @throws RuntimeException
+     */
+    protected function isProcessRunning(string $pidFile):bool
+    {
+        if (!is_file($pidFile)) {
+            return false;
+        }
+
+        try {
+            $pidFileContents = file_get_contents($pidFile);
+            if ($pidFileContents === false) {
+                throw new \Exception();
+            }
+        } catch (\Exception $e) {
+            throw new  RuntimeException(
+                "Failed to read PID file '{$pidFile}'."
+            );
+        }
+
+        $pid = trim($pidFileContents);
+
+        if (!is_numeric($pid) || (int)$pid === 0) {
+            return false;
+        }
+
+        if (PHP_OS_FAMILY === 'Windows') {
+            $output = [];
+            exec("tasklist /FI \"PID eq $pid\" 2>NUL", $output);
+
+            return str_contains(implode("\n", $output), $pid);
+        } else {
+            // Check if the process is running on POSIX (Mac or Linux)
+            exec("ps -p $pid", $output, $resultCode);
+            if ($resultCode === 0 && count($output) > 1) {
+                // Process is running
+                return true;
+            }
+        }
+
+        if (!unlink($pidFile)) {
+            throw new RuntimeException("Failed to delete PID file: $pidFile");
+        }
+
+        return false;
+    }
 }
diff --git a/tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php b/tests/unit/lucatume/WPBrowser/Extension/PidBasedControllerTest.php
@@ -0,0 +1,88 @@
+<?php
+
+
+namespace Unit\lucatume\WPBrowser\Extension;
+
+use lucatume\WPBrowser\Extension\PidBasedController;
+use lucatume\WPBrowser\Exceptions\RuntimeException;
+use Symfony\Component\Process\Process;
+
+class PidBasedControllerTest extends \Codeception\Test\Unit
+{
+    public function test_isProcessRunning_on_posix():void{
+        $testClass = new class {
+            use PidBasedController;
+
+            public function openIsProcessRunning(string $pidFile):bool{
+                return $this->isProcessRunning($pidFile);
+            }
+        };
+        $hash = md5(microtime());
+        $pidFile = sys_get_temp_dir()."/test-{$hash}.pid";
+        $pid = posix_getpid();
+        if(!file_put_contents($pidFile,$pid)){
+            $this->fail('Could not write pid to file '.$pidFile);
+        }
+
+        $this->assertTrue($testClass->openIsProcessRunning($pidFile));
+        $this->assertFileExists($pidFile);
+    }
+
+    public function test_isProcessRunning_returns_false_if_pid_file_not_exists():void{
+        $testClass = new class {
+            use PidBasedController;
+
+            public function openIsProcessRunning(string $pidFile):bool{
+                return $this->isProcessRunning($pidFile);
+            }
+        };
+        $pid = posix_getpid();
+
+        $this->assertFalse($testClass->openIsProcessRunning(__DIR__ .'/test.pid'));
+    }
+
+    public function test_isProcessRunning_throws_if_pid_file_cannot_be_read():void{
+        $testClass = new class {
+            use PidBasedController;
+
+            public function openIsProcessRunning(string $pidFile):bool{
+                return $this->isProcessRunning($pidFile);
+            }
+        };
+        $pid = posix_getpid();
+        $hash = md5(microtime());
+        $pidFile = sys_get_temp_dir()."/test-{$hash}.pid";
+        $pid = posix_getpid();
+        if(!file_put_contents($pidFile,$pid)){
+            $this->fail('Could not write pid to file '.$pidFile);
+        }
+        // Change the file mode to not be readable by the current user.
+        chmod($pidFile,0000);
+
+        $this->expectException(RuntimeException::class);
+
+        $testClass->openIsProcessRunning($pidFile);
+    }
+
+    public function test_isProcessRunning_returns_false_if_process_is_not_running():void{
+        $testClass = new class {
+            use PidBasedController;
+
+            public function openIsProcessRunning(string $pidFile):bool{
+                return $this->isProcessRunning($pidFile);
+            }
+        };
+        $hash = md5(microtime());
+        $pidFile = sys_get_temp_dir()."/test-{$hash}.pid";
+        $process = new Process(['echo', '23']);
+        $process->start();
+        $pid = $process->getPid();
+        $process->wait();
+        if(!file_put_contents($pidFile,$pid)){
+            $this->fail('Could not write pid to file '.$pidFile);
+        }
+
+        $this->assertFalse($testClass->openIsProcessRunning($pidFile));
+        $this->assertFileNotExists($pidFile);
+    }
+}
diff --git a/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php b/tests/unit/lucatume/WPBrowser/Module/WPLoaderArbitraryPluginLocationTest.php
@@ -5,7 +5,6 @@
 use Codeception\Lib\Di;
 use Codeception\Lib\ModuleContainer;
 use Codeception\Test\Unit;
-use lucatume\WPBrowser\Module\WPLoader;
 use lucatume\WPBrowser\Tests\Traits\DatabaseAssertions;
 use lucatume\WPBrowser\Tests\Traits\LoopIsolation;
 use lucatume\WPBrowser\Tests\Traits\MainInstallationAccess;
@@ -56,6 +55,72 @@ private function module(array $moduleContainerConfig = [], ?array $moduleConfig
         return new WPLoader($this->mockModuleContainer, ($moduleConfig ?? $this->config));
     }
 
+    public function test_loads_plugins_from_default_location_correctly(): void
+    {
+        $projectDir = FS::tmpDir('wploader_');
+        $installation = Installation::scaffold($projectDir);
+        $dbName = Random::dbName();
+        $dbHost = Env::get('WORDPRESS_DB_HOST');
+        $dbUser = Env::get('WORDPRESS_DB_USER');
+        $dbPassword = Env::get('WORDPRESS_DB_PASSWORD');
+        $installationDb = new MysqlDatabase($dbName, $dbUser, $dbPassword, $dbHost, 'wp_');
+        if (!mkdir($projectDir . '/wp-content/plugins/test-one', 0777, true)) {
+            throw new \RuntimeException('Unable to create test directory for plugin test-one');
+        }
+        if (!file_put_contents(
+            $projectDir . '/wp-content/plugins/test-one/plugin.php',
+            <<< PHP
+<?php
+/**
+ * Plugin Name: Test One
+ */
+ 
+ function test_one_loaded(){}
+PHP
+        )) {
+            throw new \RuntimeException('Unable to create test plugin file for plugin test-one');
+        }
+        if (!mkdir($projectDir . '/wp-content/plugins/test-two', 0777, true)) {
+            throw new \RuntimeException('Unable to create test directory for plugin test-two');
+        }
+        if (!file_put_contents(
+            $projectDir . '/wp-content/plugins/test-two/plugin.php',
+            <<< PHP
+<?php
+/**
+ * Plugin Name: Test Two
+ */
+ 
+ function test_two_loaded(){}
+PHP
+        )) {
+            throw new \RuntimeException('Unable to create test plugin file for plugin test-two');
+        }
+
+        $this->config = [
+            'wpRootFolder' => $projectDir,
+            'dbUrl' => $installationDb->getDbUrl(),
+            'tablePrefix' => 'test_',
+            'plugins' => [
+                'test-one/plugin.php',
+                'test-two/plugin.php',
+            ]
+        ];
+        $wpLoader = $this->module();
+        $projectDirname = basename($projectDir);
+
+        $this->assertInIsolation(
+            static function () use ($wpLoader, $projectDir) {
+                chdir($projectDir);
+
+                $wpLoader->_initialize();
+
+                Assert::assertTrue(function_exists('test_one_loaded'));
+                Assert::assertTrue(function_exists('test_two_loaded'));
+            }
+        );
+    }
+
     /**
      * It should allow loading a plugin from an arbitrary path
      *