-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Runtime] Automatically use FrankenPHP runner when its worker mode is detected #60503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 7.4
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Runtime\Runner; | ||
|
||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpKernel\HttpKernelInterface; | ||
use Symfony\Component\HttpKernel\TerminableInterface; | ||
use Symfony\Component\Runtime\RunnerInterface; | ||
|
||
/** | ||
* A runner for FrankenPHP in worker mode. | ||
* | ||
* @author Kévin Dunglas <kevin@dunglas.dev> | ||
*/ | ||
class FrankenPhpWorkerRunner implements RunnerInterface | ||
{ | ||
public function __construct( | ||
private HttpKernelInterface $kernel, | ||
private int $loopMax, | ||
) { | ||
} | ||
|
||
public function run(): int | ||
{ | ||
// Prevent worker script termination when a client connection is interrupted | ||
ignore_user_abort(true); | ||
|
||
$server = array_filter($_SERVER, static fn (string $key) => !str_starts_with($key, 'HTTP_'), ARRAY_FILTER_USE_KEY); | ||
$server['APP_RUNTIME_MODE'] = 'web=1&worker=1'; | ||
|
||
$handler = function () use ($server, &$sfRequest, &$sfResponse): void { | ||
Check failure on line 40 in src/Symfony/Component/Runtime/Runner/FrankenPhpWorkerRunner.php
|
||
// Connect to the Xdebug client if it's available | ||
if (\extension_loaded('xdebug') && \function_exists('xdebug_connect_to_client')) { | ||
xdebug_connect_to_client(); | ||
} | ||
|
||
// Merge the environment variables coming from DotEnv with the ones tied to the current request | ||
$_SERVER += $server; | ||
|
||
$sfRequest = Request::createFromGlobals(); | ||
$sfResponse = $this->kernel->handle($sfRequest); | ||
|
||
$sfResponse->send(); | ||
}; | ||
|
||
$loops = 0; | ||
do { | ||
$ret = \frankenphp_handle_request($handler); | ||
|
||
if ($this->kernel instanceof TerminableInterface && $sfRequest && $sfResponse) { | ||
$this->kernel->terminate($sfRequest, $sfResponse); | ||
} | ||
|
||
gc_collect_cycles(); | ||
} while ($ret && (0 >= $this->loopMax || ++$loops < $this->loopMax)); | ||
|
||
return 0; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ | |
use Symfony\Component\HttpKernel\HttpKernelInterface; | ||
use Symfony\Component\Runtime\Internal\MissingDotenv; | ||
use Symfony\Component\Runtime\Internal\SymfonyErrorHandler; | ||
use Symfony\Component\Runtime\Runner\FrankenPhpWorkerRunner; | ||
use Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner; | ||
use Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner; | ||
use Symfony\Component\Runtime\Runner\Symfony\ResponseRunner; | ||
|
@@ -42,6 +43,7 @@ class_exists(MissingDotenv::class, false) || class_exists(Dotenv::class) || clas | |
* - "use_putenv" to tell Dotenv to set env vars using putenv() (NOT RECOMMENDED.) | ||
* - "dotenv_overload" to tell Dotenv to override existing vars | ||
* - "dotenv_extra_paths" to define a list of additional dot-env files | ||
* - "worker_loop_max" to define the number of requests after which the worker must restart to prevent memory leaks | ||
* | ||
* When the "debug" / "env" options are not defined, they will fallback to the | ||
* "APP_DEBUG" / "APP_ENV" environment variables, and to the "--env|-e" / "--no-debug" | ||
|
@@ -64,6 +66,7 @@ class_exists(MissingDotenv::class, false) || class_exists(Dotenv::class) || clas | |
* - int|string|null as handled by GenericRuntime. | ||
* | ||
* @author Nicolas Grekas <p@tchwork.com> | ||
* @author Kévin Dunglas <kevin@dunglas.dev> | ||
*/ | ||
class SymfonyRuntime extends GenericRuntime | ||
{ | ||
|
@@ -73,7 +76,7 @@ class SymfonyRuntime extends GenericRuntime | |
private readonly Command $command; | ||
|
||
/** | ||
* @param array { | ||
* @param array{ | ||
* debug?: ?bool, | ||
* env?: ?string, | ||
* disable_dotenv?: ?bool, | ||
|
@@ -88,6 +91,7 @@ class SymfonyRuntime extends GenericRuntime | |
* debug_var_name?: string, | ||
* dotenv_overload?: ?bool, | ||
* dotenv_extra_paths?: ?string[], | ||
* worker_loop_max?: int, // Use 0 or a negative integer to never restart the worker. Default: 500 | ||
* } $options | ||
*/ | ||
public function __construct(array $options = []) | ||
|
@@ -143,12 +147,23 @@ public function __construct(array $options = []) | |
|
||
$options['error_handler'] ??= SymfonyErrorHandler::class; | ||
|
||
$workerLoopMax = $options['worker_loop_max'] ?? $_SERVER['FRANKENPHP_LOOP_MAX'] ?? $_ENV['FRANKENPHP_LOOP_MAX'] ?? null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the env var has the same name as the option? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kept as is to keep the backward compatibility with the current runtime package, but maybe it's the occasion to have a better name indeed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'd say we can keep FRANKENPHP_LOOP_MAP to help transitioning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't that require a modification in FrankenPHP itself? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it requires an update, we could make FrankenPHP compatible with both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to modify FrankenPHP, this option is entirely handled by the runtime. |
||
if (null !== $workerLoopMax && null === filter_var($workerLoopMax, \FILTER_VALIDATE_INT, \FILTER_NULL_ON_FAILURE)) { | ||
throw new \LogicException(\sprintf('The "worker_loop_max" runtime option must be an integer, "%s" given.', get_debug_type($workerLoopMax))); | ||
} | ||
|
||
$options['worker_loop_max'] = (int) ($workerLoopMax ?? 500); | ||
|
||
parent::__construct($options); | ||
} | ||
|
||
public function getRunner(?object $application): RunnerInterface | ||
{ | ||
if ($application instanceof HttpKernelInterface) { | ||
if ($_SERVER['FRANKENPHP_WORKER'] ?? false) { | ||
nicolas-grekas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return new FrankenPhpWorkerRunner($application, $this->options['worker_loop_max']); | ||
} | ||
|
||
return new HttpKernelRunner($application, Request::createFromGlobals(), $this->options['debug'] ?? false); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Runtime\Tests; | ||
|
||
require_once __DIR__.'/frankenphp-function-mock.php'; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Symfony\Component\HttpKernel\HttpKernelInterface; | ||
use Symfony\Component\HttpKernel\TerminableInterface; | ||
use Symfony\Component\Runtime\Runner\FrankenPhpWorkerRunner; | ||
|
||
interface TestAppInterface extends HttpKernelInterface, TerminableInterface | ||
{ | ||
} | ||
|
||
/** | ||
* @author Kévin Dunglas <kevin@dunglas.fr> | ||
*/ | ||
class FrankenPhpWorkerRunnerTest extends TestCase | ||
{ | ||
public function testRun() | ||
{ | ||
$application = $this->createMock(TestAppInterface::class); | ||
$application | ||
->expects($this->once()) | ||
->method('handle') | ||
->willReturnCallback(function (Request $request, int $type = HttpKernelInterface::MAIN_REQUEST, bool $catch = true): Response { | ||
$this->assertSame('bar', $request->server->get('FOO')); | ||
|
||
return new Response(); | ||
}); | ||
$application->expects($this->once())->method('terminate'); | ||
|
||
$_SERVER['FOO'] = 'bar'; | ||
|
||
$runner = new FrankenPhpWorkerRunner($application, 500); | ||
$this->assertSame(0, $runner->run()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
namespace Symfony\Component\Runtime\Tests; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Symfony\Component\HttpKernel\HttpKernelInterface; | ||
use Symfony\Component\Runtime\Runner\FrankenPhpWorkerRunner; | ||
use Symfony\Component\Runtime\SymfonyRuntime; | ||
|
||
/** | ||
* @author Kévin Dunglas <kevin@dunglas.dev> | ||
* @author Alexandre Daubois <alex.daubois@gmail.com> | ||
*/ | ||
class SymfonyRuntimeTest extends TestCase | ||
{ | ||
public function testGetRunner() | ||
{ | ||
$application = $this->createStub(HttpKernelInterface::class); | ||
|
||
$runtime = new SymfonyRuntime(); | ||
$this->assertNotInstanceOf(FrankenPhpWorkerRunner::class, $runtime->getRunner(null)); | ||
$this->assertNotInstanceOf(FrankenPhpWorkerRunner::class, $runtime->getRunner($application)); | ||
|
||
$_SERVER['FRANKENPHP_WORKER'] = 1; | ||
$this->assertInstanceOf(FrankenPhpWorkerRunner::class, $runtime->getRunner($application)); | ||
} | ||
|
||
public function testStringWorkerMaxLoopThrows() | ||
{ | ||
$this->expectException(\LogicException::class); | ||
$this->expectExceptionMessage('The "worker_loop_max" runtime option must be an integer, "string" given.'); | ||
|
||
new SymfonyRuntime(['worker_loop_max' => 'foo']); | ||
} | ||
|
||
public function testBoolWorkerMaxLoopThrows() | ||
{ | ||
$this->expectException(\LogicException::class); | ||
$this->expectExceptionMessage('The "worker_loop_max" runtime option must be an integer, "bool" given.'); | ||
|
||
new SymfonyRuntime(['worker_loop_max' => false]); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
/* | ||
* This file is part of the Symfony package. | ||
* | ||
* (c) Fabien Potencier <fabien@symfony.com> | ||
* | ||
* For the full copyright and license information, please view the LICENSE | ||
* file that was distributed with this source code. | ||
*/ | ||
|
||
if (!function_exists('frankenphp_handle_request')) { | ||
function frankenphp_handle_request(callable $callable): bool | ||
{ | ||
$callable(); | ||
|
||
return false; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering about this: could it become a perf hog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe could we make this configurable (ex: run it after N requests), but this prevents the GC to be triggered in the middle of the handling of a request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worker mode is relatively new to PHP so maybe there's something to improve on this topic in php-src itself?
Some new conditional trigger that'd be light to check when running the GC would be really useful?
Or maybe we can build something on gc_status?
If we want to give more control over this to end users, we could move the call into a kernel-terminate listener, that'd do it only in worker mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to move this logic to a listener and make it configurable, but maybe it could be done in a follow-up PR?