Merge pull request #272 from mnapoli/kill-dangling-fpm

Fix #265 Recover from Lambda timeouts

demo/http.php

@@ -1,20 +1,9 @@
 <?php declare(strict_types=1);
 
-use Psr\Http\Message\ResponseInterface;
-use Psr\Http\Message\ServerRequestInterface;
-
 require __DIR__ . '/../vendor/autoload.php';
 
-$slim = new Slim\App;
-
-$slim->get('/', function (ServerRequestInterface $request, ResponseInterface $response) {
-    $response->getBody()->write('Hello world!');
-    return $response;
-});
-$slim->get('/json', function (ServerRequestInterface $request, ResponseInterface $response) {
-    $response->getBody()->write(json_encode(['hello' => 'json']));
-    $response = $response->withHeader('Content-Type', 'application/json');
-    return $response;
-});
+if (isset($_GET['sleep'])) {
+    sleep(10);
+}
 
-$slim->run();
+echo 'Hello world!';

phpstan.neon.dist

@@ -12,3 +12,7 @@ parameters:
     - %rootDir%/../../../tests/Bridge/Symfony/logs/*
     - %rootDir%/../../../tests/Sam/Php/*
     - %rootDir%/../../../tests/Sam/PhpFpm/*
+
+  ignoreErrors:
+    # https://github.com/phpstan/phpstan/pull/2002
+    - '#Strict comparison using === between int and false will always evaluate to false#'

runtime/php/layers/fpm/php-fpm.conf

@@ -1,5 +1,6 @@
 ; Logging anywhere on disk doesn't make sense on lambda since instances are ephemeral
 error_log = /dev/null
+pid = /tmp/.bref/php-fpm.pid
 ; Log above warning because PHP-FPM logs useless notices
 ; We must comment this flag else uncaught exceptions/fatal errors are not reported in the logs!
 ; TODO: report that to the PHP bug tracker

src/Runtime/PhpFpm.php

@@ -24,6 +24,7 @@
 class PhpFpm
 {
     private const SOCKET = '/tmp/.bref/php-fpm.sock';
+    private const PID_FILE = '/tmp/.bref/php-fpm.pid';
     private const CONFIG = '/opt/bref/etc/php-fpm.conf';
 
     /** @var Client|null */
@@ -46,8 +47,10 @@ public function __construct(string $handler, string $configFile = self::CONFIG)
      */
     public function start(): void
     {
+        // In case Lambda stopped our process (e.g. because of a timeout) we need to make sure PHP-FPM has stopped
+        // as well and restart it
         if ($this->isReady()) {
-            throw new \Exception('PHP-FPM has already been started, aborting');
+            $this->killExistingFpm();
         }
 
         if (! is_dir(dirname(self::SOCKET))) {
@@ -67,7 +70,7 @@ public function start(): void
 
         $this->reconnect();
 
-        $this->waitForServerReady();
+        $this->waitUntilReady();
     }
 
     public function stop(): void
@@ -140,7 +143,7 @@ public function proxy($event): LambdaResponse
         return new LambdaResponse((int) $status, $responseHeaders, $responseBody);
     }
 
-    private function waitForServerReady(): void
+    private function waitUntilReady(): void
     {
         $wait = 5000; // 5ms
         $timeout = 5000000; // 5 secs
@@ -252,4 +255,66 @@ private function reconnect(): void
 
         $this->client = new Client('unix://' . self::SOCKET, 30000);
     }
+
+    /**
+     * This methods makes sure to kill any existing PHP-FPM process.
+     */
+    private function killExistingFpm(): void
+    {
+        // Never seen this happen but just in case
+        if (! file_exists(self::PID_FILE)) {
+            unlink(self::SOCKET);
+            return;
+        }
+
+        $pid = (int) file_get_contents(self::PID_FILE);
+
+        // Never seen this happen but just in case
+        if ($pid <= 0) {
+            echo "PHP-FPM's PID file contained an invalid PID, assuming PHP-FPM isn't running.\n";
+            unlink(self::SOCKET);
+            unlink(self::PID_FILE);
+            return;
+        }
+
+        // Check if the process is running
+        if (posix_getpgid($pid) === false) {
+            // PHP-FPM is not running anymore, we can cleanup
+            unlink(self::SOCKET);
+            unlink(self::PID_FILE);
+            return;
+        }
+
+        echo "PHP-FPM seems to be running already, this might be because Lambda stopped the bootstrap process but didn't leave us an opportunity to stop PHP-FPM. Stopping PHP-FPM now to restart from a blank slate.\n";
+
+        // PHP-FPM is running, let's try to kill it properly
+        $result = posix_kill($pid, SIGTERM);
+        if ($result === false) {
+            echo "PHP-FPM's PID file contained a PID that doesn't exist, assuming PHP-FPM isn't running.\n";
+            unlink(self::SOCKET);
+            unlink(self::PID_FILE);
+            return;
+        }
+
+        $this->waitUntilStopped($pid);
+        unlink(self::SOCKET);
+        unlink(self::PID_FILE);
+    }
+
+    /**
+     * Wait until PHP-FPM has stopped.
+     */
+    private function waitUntilStopped(int $pid): void
+    {
+        $wait = 5000; // 5ms
+        $timeout = 1000000; // 1 sec
+        $elapsed = 0;
+        while (posix_getpgid($pid) !== false) {
+            usleep($wait);
+            $elapsed += $wait;
+            if ($elapsed > $timeout) {
+                throw new \Exception('Timeout while waiting for PHP-FPM to stop');
+            }
+        }
+    }
 }

template.yaml

@@ -25,7 +25,7 @@ Resources:
             Description: 'Bref HTTP demo'
             CodeUri: .
             Handler: demo/http.php
-            Timeout: 30 # in seconds (API Gateway has a timeout of 30 seconds)
+            Timeout: 5 # in seconds (API Gateway has a timeout of 30 seconds)
             MemorySize: 1024 # The memory size is related to the pricing and CPU power
             Runtime: provided
             Layers:

tests/Runtime/PhpFpmTest.php

@@ -701,7 +701,10 @@ public function test response with error_log()
         ], $response['headers']);
     }
 
-    public function test timeouts are recovered from()
+    /**
+     * Checks that a timeout cause by the PHP-FPM limit (not the Lambda limit) can be recovered from
+     */
+    public function test FPM timeouts are recovered from()
     {
         $this->fpm = new PhpFpm(__DIR__ . '/PhpFpm/timeout.php', __DIR__ . '/PhpFpm/php-fpm.conf');
         $this->fpm->start();