From 1907014122d84ffdba5b964954846bab7d9da1dc Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Wed, 26 Jan 2022 18:12:54 +0000 Subject: [PATCH 1/3] Fixes app instance memory leak between feature tests --- .../Foundation/Bootstrap/HandleExceptions.php | 44 ++++++++++++++----- .../Foundation/Testing/TestCase.php | 3 ++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index ced85fea5058..aa300117d7b6 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -26,7 +26,7 @@ class HandleExceptions * * @var \Illuminate\Contracts\Foundation\Application */ - protected $app; + protected static $app; /** * Bootstrap the given application. @@ -38,15 +38,15 @@ public function bootstrap(Application $app) { self::$reservedMemory = str_repeat('x', 10240); - $this->app = $app; + static::$app = $app; error_reporting(-1); - set_error_handler([$this, 'handleError']); + set_error_handler($this->forwardsTo('handleError')); - set_exception_handler([$this, 'handleException']); + set_exception_handler($this->forwardsTo('handleException')); - register_shutdown_function([$this, 'handleShutdown']); + register_shutdown_function($this->forwardsTo('handleShutdown')); if (! $app->environment('testing')) { ini_set('display_errors', 'Off'); @@ -91,7 +91,7 @@ public function handleDeprecation($message, $file, $line) } try { - $logger = $this->app->make(LogManager::class); + $logger = static::$app->make(LogManager::class); } catch (Exception $e) { return; } @@ -112,7 +112,7 @@ public function handleDeprecation($message, $file, $line) */ protected function ensureDeprecationLoggerIsConfigured() { - with($this->app['config'], function ($config) { + with(static::$app['config'], function ($config) { if ($config->get('logging.channels.deprecations')) { return; } @@ -132,7 +132,7 @@ protected function ensureDeprecationLoggerIsConfigured() */ protected function ensureNullLogDriverIsConfigured() { - with($this->app['config'], function ($config) { + with(static::$app['config'], function ($config) { if ($config->get('logging.channels.null')) { return; } @@ -164,7 +164,7 @@ public function handleException(Throwable $e) // } - if ($this->app->runningInConsole()) { + if (static::$app->runningInConsole()) { $this->renderForConsole($e); } else { $this->renderHttpResponse($e); @@ -190,7 +190,7 @@ protected function renderForConsole(Throwable $e) */ protected function renderHttpResponse(Throwable $e) { - $this->getExceptionHandler()->render($this->app['request'], $e)->send(); + $this->getExceptionHandler()->render(static::$app['request'], $e)->send(); } /** @@ -217,6 +217,18 @@ protected function fatalErrorFromPhpError(array $error, $traceOffset = null) return new FatalError($error['message'], 0, $error, $traceOffset); } + /** + * Forward a method call to the given method, if an app instance exists. + * + * @return callable + */ + protected function forwardsTo($method) + { + return fn (...$arguments) => static::$app + ? $this->{$method}(...$arguments) + : false; + } + /** * Determine if the error level is a deprecation. * @@ -246,6 +258,16 @@ protected function isFatal($type) */ protected function getExceptionHandler() { - return $this->app->make(ExceptionHandler::class); + return static::$app->make(ExceptionHandler::class); + } + + /** + * Clears the app instance. + * + * @return void + */ + public static function forgetApp() + { + static::$app = null; } } diff --git a/src/Illuminate/Foundation/Testing/TestCase.php b/src/Illuminate/Foundation/Testing/TestCase.php index 35af3d49bc0e..9d8c514d2fec 100644 --- a/src/Illuminate/Foundation/Testing/TestCase.php +++ b/src/Illuminate/Foundation/Testing/TestCase.php @@ -5,6 +5,7 @@ use Carbon\CarbonImmutable; use Illuminate\Console\Application as Artisan; use Illuminate\Database\Eloquent\Model; +use Illuminate\Foundation\Bootstrap\HandleExceptions; use Illuminate\Queue\Queue; use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Facade; @@ -203,6 +204,8 @@ protected function tearDown(): void Queue::createPayloadUsing(null); + HandleExceptions::forgetApp(); + if ($this->callbackException) { throw $this->callbackException; } From 8ad709f3c40162d12ef8208a24bf5ab4a53deee5 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Fri, 28 Jan 2022 14:38:29 +0000 Subject: [PATCH 2/3] Adds tests --- .../Bootstrap/HandleExceptionsTest.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/Foundation/Bootstrap/HandleExceptionsTest.php b/tests/Foundation/Bootstrap/HandleExceptionsTest.php index 6bb1ee150bda..4af5fb06632c 100644 --- a/tests/Foundation/Bootstrap/HandleExceptionsTest.php +++ b/tests/Foundation/Bootstrap/HandleExceptionsTest.php @@ -5,6 +5,7 @@ use ErrorException; use Illuminate\Config\Repository as Config; use Illuminate\Container\Container; +use Illuminate\Contracts\Foundation\Application; use Illuminate\Foundation\Bootstrap\HandleExceptions; use Illuminate\Log\LogManager; use Mockery as m; @@ -208,6 +209,39 @@ public function testIgnoreDeprecationIfLoggerUnresolvable() 17 ); } + + public function testForgetApp() + { + $appResolver = fn () => with(new ReflectionClass($this->handleExceptions), function ($reflection) { + $property = tap($reflection->getProperty('app'))->setAccessible(true); + + return $property->getValue($this->handleExceptions); + }); + + $this->assertNotNull($appResolver()); + + handleExceptions::forgetApp(); + + $this->assertNull($appResolver()); + } + + public function testHandlerForgetsPreviousApp() + { + $appResolver = fn () => with(new ReflectionClass($this->handleExceptions), function ($reflection) { + $property = tap($reflection->getProperty('app'))->setAccessible(true); + + return $property->getValue($this->handleExceptions); + }); + + $this->assertSame($this->container, $appResolver()); + + $this->handleExceptions->bootstrap($newApp = tap(m::mock(Application::class), function ($app) { + $app->shouldReceive('environment')->once()->andReturn(true); + })); + + $this->assertNotSame($this->container, $appResolver()); + $this->assertSame($newApp, $appResolver()); + } } class CustomNullHandler extends NullHandler From 08a6069fcf14d4cdb3662806f9e999a10ecd8b44 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Fri, 28 Jan 2022 15:26:03 -0600 Subject: [PATCH 3/3] formatting --- src/Illuminate/Foundation/Bootstrap/HandleExceptions.php | 4 ++-- src/Illuminate/Foundation/Testing/TestCase.php | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php index aa300117d7b6..e5b2123078e0 100644 --- a/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php +++ b/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php @@ -218,7 +218,7 @@ protected function fatalErrorFromPhpError(array $error, $traceOffset = null) } /** - * Forward a method call to the given method, if an app instance exists. + * Forward a method call to the given method if an application instance exists. * * @return callable */ @@ -262,7 +262,7 @@ protected function getExceptionHandler() } /** - * Clears the app instance. + * Clear the local application instance from memory. * * @return void */ diff --git a/src/Illuminate/Foundation/Testing/TestCase.php b/src/Illuminate/Foundation/Testing/TestCase.php index 9d8c514d2fec..d7e92b557288 100644 --- a/src/Illuminate/Foundation/Testing/TestCase.php +++ b/src/Illuminate/Foundation/Testing/TestCase.php @@ -201,9 +201,7 @@ protected function tearDown(): void $this->beforeApplicationDestroyedCallbacks = []; Artisan::forgetBootstrappers(); - Queue::createPayloadUsing(null); - HandleExceptions::forgetApp(); if ($this->callbackException) {