From d6f23893b5735500095bc65743a51d84a1d2adc5 Mon Sep 17 00:00:00 2001 From: Baptiste Langlade Date: Sat, 26 Oct 2024 14:53:08 +0200 Subject: [PATCH] use static closures to avoid capturing $this to avoid circular references --- CHANGELOG.md | 6 +++++ src/Application/Async/Http.php | 46 ++++++++++++++++++++++++---------- src/Application/Cli.php | 19 +++++++++----- src/Application/Http.php | 24 +++++++++++------- src/Http/Router.php | 3 ++- src/Middleware/LoadDotEnv.php | 10 +++++--- 6 files changed, 75 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6906d4..c77b8a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [Unreleased] + +### Changed + +- Use `static` closures as much as possible to reduce the probability of creating circular references by capturing `$this` as it can lead to memory root buffer exhaustion. + ## 2.2.0 - 2024-03-24 ### Added diff --git a/src/Application/Async/Http.php b/src/Application/Async/Http.php index 583bc73..135e0b7 100644 --- a/src/Application/Async/Http.php +++ b/src/Application/Async/Http.php @@ -100,10 +100,12 @@ public static function of(OperatingSystem $os): self */ public function mapEnvironment(callable $map): self { + $previous = $this->map; + return new self( $this->os, - function(OperatingSystem $os, Environment $env) use ($map): array { - [$os, $env] = ($this->map)($os, $env); + static function(OperatingSystem $os, Environment $env) use ($previous, $map): array { + [$os, $env] = $previous($os, $env); $env = $map($env, $os); return [$os, $env]; @@ -120,10 +122,12 @@ function(OperatingSystem $os, Environment $env) use ($map): array { */ public function mapOperatingSystem(callable $map): self { + $previous = $this->map; + return new self( $this->os, - function(OperatingSystem $os, Environment $env) use ($map): array { - [$os, $env] = ($this->map)($os, $env); + static function(OperatingSystem $os, Environment $env) use ($previous, $map): array { + [$os, $env] = $previous($os, $env); $os = $map($os, $env); return [$os, $env]; @@ -140,10 +144,12 @@ function(OperatingSystem $os, Environment $env) use ($map): array { */ public function service(string|Service $name, callable $definition): self { + $container = $this->container; + return new self( $this->os, $this->map, - fn(OperatingSystem $os, Environment $env) => ($this->container)($os, $env)->add( + static fn(OperatingSystem $os, Environment $env) => $container($os, $env)->add( $name, static fn($service) => $definition($service, $os, $env), ), @@ -207,18 +213,20 @@ public function appendRoutes(callable $append): self */ public function mapRequestHandler(callable $map): self { + $previous = $this->mapRequestHandler; + return new self( $this->os, $this->map, $this->container, $this->routes, - fn( + static fn( RequestHandler $handler, Container $container, OperatingSystem $os, Environment $env, ) => $map( - ($this->mapRequestHandler)($handler, $container, $os, $env), + $previous($handler, $container, $os, $env), $container, $os, $env, @@ -244,13 +252,25 @@ public function notFoundRequestHandler(callable $handle): self public function run($input) { + $map = $this->map; + $container = $this->container; + $routes = $this->routes; + $notFound = $this->notFound; + $mapRequestHandler = $this->mapRequestHandler; + $run = Commands::of(Serve::of( $this->os, - function(ServerRequest $request, OperatingSystem $os): Response { + static function(ServerRequest $request, OperatingSystem $os) use ( + $map, + $container, + $routes, + $notFound, + $mapRequestHandler, + ): Response { $env = Environment::http($request->environment()); - [$os, $env] = ($this->map)($os, $env); - $container = ($this->container)($os, $env)->build(); - $routes = Sequence::lazyStartingWith($this->routes) + [$os, $env] = $map($os, $env); + $container = $container($os, $env)->build(); + $routes = Sequence::lazyStartingWith($routes) ->flatMap(static fn($routes) => $routes) ->map(static fn($provide) => $provide( Routes::lazy(), @@ -261,7 +281,7 @@ function(ServerRequest $request, OperatingSystem $os): Response { ->flatMap(static fn($routes) => $routes->toSequence()); $router = new Router( $routes, - $this->notFound->map( + $notFound->map( static fn($handle) => static fn(ServerRequest $request) => $handle( $request, $container, @@ -270,7 +290,7 @@ function(ServerRequest $request, OperatingSystem $os): Response { ), ), ); - $handle = ($this->mapRequestHandler)($router, $container, $os, $env); + $handle = $mapRequestHandler($router, $container, $os, $env); return $handle($request); }, diff --git a/src/Application/Cli.php b/src/Application/Cli.php index f2b1825..31c5254 100644 --- a/src/Application/Cli.php +++ b/src/Application/Cli.php @@ -109,10 +109,12 @@ public function mapOperatingSystem(callable $map): self */ public function service(string|Service $name, callable $definition): self { + $container = $this->container; + return new self( $this->os, $this->env, - fn(OperatingSystem $os, Environment $env) => ($this->container)($os, $env)->add( + static fn(OperatingSystem $os, Environment $env) => $container($os, $env)->add( $name, static fn($service) => $definition($service, $os, $env), ), @@ -144,18 +146,20 @@ public function command(callable $command): self */ public function mapCommand(callable $map): self { + $previous = $this->mapCommand; + return new self( $this->os, $this->env, $this->container, $this->commands, - fn( + static fn( Command $command, Container $service, OperatingSystem $os, Environment $env, ) => $map( - ($this->mapCommand)($command, $service, $os, $env), + $previous($command, $service, $os, $env), $service, $os, $env, @@ -198,11 +202,14 @@ public function notFoundRequestHandler(callable $handle): self public function run($input) { $container = ($this->container)($this->os, $this->env)->build(); - $mapCommand = fn(Command $command): Command => ($this->mapCommand)( + $mapCommand = $this->mapCommand; + $os = $this->os; + $env = $this->env; + $mapCommand = static fn(Command $command): Command => $mapCommand( $command, $container, - $this->os, - $this->env, + $os, + $env, ); $commands = $this->commands->map(static fn($service) => new Defer( $service, diff --git a/src/Application/Http.php b/src/Application/Http.php index 3a2f86f..8596160 100644 --- a/src/Application/Http.php +++ b/src/Application/Http.php @@ -121,10 +121,12 @@ public function mapOperatingSystem(callable $map): self */ public function service(string|Service $name, callable $definition): self { + $container = $this->container; + return new self( $this->os, $this->env, - fn(OperatingSystem $os, Environment $env) => ($this->container)($os, $env)->add( + static fn(OperatingSystem $os, Environment $env) => $container($os, $env)->add( $name, static fn($service) => $definition($service, $os, $env), ), @@ -188,18 +190,20 @@ public function appendRoutes(callable $append): self */ public function mapRequestHandler(callable $map): self { + $previous = $this->mapRequestHandler; + return new self( $this->os, $this->env, $this->container, $this->routes, - fn( + static fn( RequestHandler $handler, Container $container, OperatingSystem $os, Environment $env, ) => $map( - ($this->mapRequestHandler)($handler, $container, $os, $env), + $previous($handler, $container, $os, $env), $container, $os, $env, @@ -226,23 +230,25 @@ public function notFoundRequestHandler(callable $handle): self public function run($input) { $container = ($this->container)($this->os, $this->env)->build(); + $os = $this->os; + $env = $this->env; $routes = Sequence::lazyStartingWith($this->routes) ->flatMap(static fn($routes) => $routes) - ->map(fn($provide) => $provide( + ->map(static fn($provide) => $provide( Routes::lazy(), $container, - $this->os, - $this->env, + $os, + $env, )) ->flatMap(static fn($routes) => $routes->toSequence()); $router = new Router( $routes, $this->notFound->map( - fn($handle) => fn(ServerRequest $request) => $handle( + static fn($handle) => static fn(ServerRequest $request) => $handle( $request, $container, - $this->os, - $this->env, + $os, + $env, ), ), ); diff --git a/src/Http/Router.php b/src/Http/Router.php index b624605..55904ea 100644 --- a/src/Http/Router.php +++ b/src/Http/Router.php @@ -41,10 +41,11 @@ public function __construct(Sequence $routes, Maybe $notFound) public function __invoke(ServerRequest $request): Response { $match = new RequestMatcher($this->routes); + $notFound = $this->notFound; return $match($request) ->map(static fn($route) => $route->respondTo(...)) - ->otherwise(fn() => $this->notFound) + ->otherwise(static fn() => $notFound) ->match( static fn($handle) => $handle($request), static fn() => Response::of( diff --git a/src/Middleware/LoadDotEnv.php b/src/Middleware/LoadDotEnv.php index de41b43..15e8389 100644 --- a/src/Middleware/LoadDotEnv.php +++ b/src/Middleware/LoadDotEnv.php @@ -30,14 +30,16 @@ private function __construct(Path $folder) public function __invoke(Application $app): Application { + $folder = $this->folder; + return $app->mapEnvironment( - fn($env, $os) => $os + static fn($env, $os) => $os ->filesystem() - ->mount($this->folder) + ->mount($folder) ->get(Name::of('.env')) ->keep(Instance::of(File::class)) ->match( - fn($file) => $this->add($env, $file), + static fn($file) => self::add($env, $file), static fn() => $env, ), ); @@ -48,7 +50,7 @@ public static function at(Path $folder): self return new self($folder); } - private function add(Environment $env, File $file): Environment + private static function add(Environment $env, File $file): Environment { /** @psalm-suppress InvalidArgument Due to the empty sequence in the flatMap */ return $file