diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b8e8c43b..153d44d88 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,8 +74,11 @@ jobs: run: composer update --no-progress --no-interaction --prefer-dist --prefer-lowest if: ${{ matrix.dependencies == 'lowest' }} - - name: Run tests - run: vendor/bin/phpunit --coverage-clover=coverage.xml + - name: Run unit tests + run: vendor/bin/phpunit --testsuite unit --coverage-clover=coverage.xml + # The reason for running some OOM tests without coverage is that because the coverage information collector can cause another OOM event invalidating the test + - name: Run out of memory tests (without coverage) + run: vendor/bin/phpunit --testsuite oom --no-coverage - name: Upload code coverage uses: codecov/codecov-action@v3 diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 0f5d14742..d064f8775 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -12,10 +12,14 @@ - + tests tests/phpt + + + tests/phpt-oom + diff --git a/src/ErrorHandler.php b/src/ErrorHandler.php index fc6ff3253..ef558d919 100644 --- a/src/ErrorHandler.php +++ b/src/ErrorHandler.php @@ -22,7 +22,14 @@ final class ErrorHandler * * @internal */ - public const DEFAULT_RESERVED_MEMORY_SIZE = 10240; + public const DEFAULT_RESERVED_MEMORY_SIZE = 16 * 1024; // 16 KiB + + /** + * The regular expression used to match the message of an out of memory error. + * + * Regex inspired by https://github.com/php/php-src/blob/524b13460752fba908f88e3c4428b91fa66c083a/Zend/tests/new_oom.phpt#L15 + */ + private const OOM_MESSAGE_MATCHER = '/^Allowed memory size of (?\d+) bytes exhausted[^\r\n]* \(tried to allocate \d+ bytes\)/'; /** * The fatal error types that cannot be silenced using the @ operator in PHP 8+. @@ -89,11 +96,27 @@ final class ErrorHandler private $isFatalErrorHandlerRegistered = false; /** - * @var string|null A portion of pre-allocated memory data that will be reclaimed - * in case a fatal error occurs to handle it + * @var int|null the amount of bytes of memory to increase the memory limit by when we are capturing a out of memory error, set to null to not increase the memory limit + */ + private $memoryLimitIncreaseOnOutOfMemoryErrorValue = 5 * 1024 * 1024; // 5 MiB + + /** + * @var bool Whether the memory limit has been increased + */ + private static $didIncreaseMemoryLimit = false; + + /** + * @var string|null A portion of pre-allocated memory data that will be reclaimed in case a fatal error occurs to handle it + * + * @phpstan-ignore-next-line This property is used to reserve memory for the fatal error handler and is thus never read */ private static $reservedMemory; + /** + * @var bool Whether the fatal error handler should be disabled + */ + private static $disableFatalErrorHandler = false; + /** * @var string[] List of error levels and their description */ @@ -254,6 +277,20 @@ public function addExceptionHandlerListener(callable $listener): void $this->exceptionListeners[] = $listener; } + /** + * Sets the amount of memory to increase the memory limit by when we are capturing a out of memory error. + * + * @param int|null $valueInBytes the number of bytes to increase the memory limit by, or null to not increase the memory limit + */ + public function setMemoryLimitIncreaseOnOutOfMemoryErrorInBytes(?int $valueInBytes): void + { + if ($valueInBytes !== null && $valueInBytes <= 0) { + throw new \InvalidArgumentException('The $valueInBytes argument must be greater than 0 or null.'); + } + + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue = $valueInBytes; + } + /** * Handles errors by capturing them through the client according to the * configured bit field. @@ -315,16 +352,28 @@ private function handleError(int $level, string $message, string $file, int $lin */ private function handleFatalError(): void { - // If there is not enough memory that can be used to handle the error - // do nothing - if (self::$reservedMemory === null) { + if (self::$disableFatalErrorHandler) { return; } + // Free the reserved memory that allows us to potentially handle OOM errors self::$reservedMemory = null; + $error = error_get_last(); if (!empty($error) && $error['type'] & (\E_ERROR | \E_PARSE | \E_CORE_ERROR | \E_CORE_WARNING | \E_COMPILE_ERROR | \E_COMPILE_WARNING)) { + // If we did not do so already and we are allowed to increase the memory limit, we do so when we detect an OOM error + if (self::$didIncreaseMemoryLimit === false + && $this->memoryLimitIncreaseOnOutOfMemoryErrorValue !== null + && preg_match(self::OOM_MESSAGE_MATCHER, $error['message'], $matches) === 1 + ) { + $currentMemoryLimit = (int) $matches['memory_limit']; + + ini_set('memory_limit', (string) ($currentMemoryLimit + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue)); + + self::$didIncreaseMemoryLimit = true; + } + $errorAsException = new FatalErrorException(self::ERROR_LEVELS_DESCRIPTION[$error['type']] . ': ' . $error['message'], 0, $error['type'], $error['file'], $error['line']); $this->exceptionReflection->setValue($errorAsException, []); @@ -369,7 +418,7 @@ private function handleException(\Throwable $exception): void // native PHP handler to prevent an infinite loop if ($exception === $previousExceptionHandlerException) { // Disable the fatal error handler or the error will be reported twice - self::$reservedMemory = null; + self::$disableFatalErrorHandler = true; throw $exception; } diff --git a/tests/phpt-oom/out_of_memory_fatal_error_increases_memory_limit.phpt b/tests/phpt-oom/out_of_memory_fatal_error_increases_memory_limit.phpt new file mode 100644 index 000000000..acef1edd9 --- /dev/null +++ b/tests/phpt-oom/out_of_memory_fatal_error_increases_memory_limit.phpt @@ -0,0 +1,77 @@ +--TEST-- +Test that when handling a out of memory error the memory limit is increased with 5 MiB and the event is serialized and ready to be sent +--INI-- +memory_limit=67108864 +--FILE-- + 'http://public@example.com/sentry/1', +]); + +$transport = new class(new PayloadSerializer($options)) implements TransportInterface { + private $payloadSerializer; + + public function __construct(PayloadSerializerInterface $payloadSerializer) + { + $this->payloadSerializer = $payloadSerializer; + } + + public function send(Event $event): Result + { + $serialized = $this->payloadSerializer->serialize($event); + + echo 'Transport called' . \PHP_EOL; + + return new Result(ResultStatus::success()); + } + + public function close(?int $timeout = null): Result + { + return new Result(ResultStatus::success()); + } +}; + +$options->setTransport($transport); + +$client = (new ClientBuilder($options))->getClient(); + +SentrySdk::init()->bindClient($client); + +echo 'Before OOM memory limit: ' . \ini_get('memory_limit'); + +register_shutdown_function(function () { + echo 'After OOM memory limit: ' . \ini_get('memory_limit'); +}); + +$array = []; +for ($i = 0; $i < 100000000; ++$i) { + $array[] = 'sentry'; +} +--EXPECTF-- +Before OOM memory limit: 67108864 +Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d +Transport called +After OOM memory limit: 72351744 diff --git a/tests/phpt/error_handler_captures_out_of_memory_fatal_error.phpt b/tests/phpt/error_handler_captures_out_of_memory_fatal_error.phpt index 0c788d88b..bcc62e32b 100644 --- a/tests/phpt/error_handler_captures_out_of_memory_fatal_error.phpt +++ b/tests/phpt/error_handler_captures_out_of_memory_fatal_error.phpt @@ -1,7 +1,7 @@ --TEST-- -Test catching out of memory fatal error +Test catching out of memory fatal error without increasing memory limit --INI-- -memory_limit=128M +memory_limit=67108864 --FILE-- addErrorHandlerListener(static function (): void { - echo 'Error listener called (it should not have been)' . PHP_EOL; -}); - -$errorHandler = ErrorHandler::registerOnceFatalErrorHandler(1024 * 1024); +$errorHandler = ErrorHandler::registerOnceFatalErrorHandler(); $errorHandler->addFatalErrorHandlerListener(static function (): void { echo 'Fatal error listener called' . PHP_EOL; -}); -$errorHandler = ErrorHandler::registerOnceExceptionHandler(); -$errorHandler->addExceptionHandlerListener(static function (): void { - echo 'Exception listener called (it should not have been)' . PHP_EOL; + echo 'After OOM memory limit: ' . ini_get('memory_limit'); }); +$errorHandler->setMemoryLimitIncreaseOnOutOfMemoryErrorInBytes(null); + +echo 'Before OOM memory limit: ' . ini_get('memory_limit'); + $foo = str_repeat('x', 1024 * 1024 * 1024); ?> --EXPECTF-- +Before OOM memory limit: 67108864 Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d Fatal error listener called +After OOM memory limit: 67108864 diff --git a/tests/phpt/error_handler_handles_exception_only_once.phpt b/tests/phpt/error_handler_handles_exception_only_once.phpt new file mode 100644 index 000000000..b57432a79 --- /dev/null +++ b/tests/phpt/error_handler_handles_exception_only_once.phpt @@ -0,0 +1,36 @@ +--TEST-- +Test that exceptions are only handled once +--FILE-- +addFatalErrorHandlerListener(static function (): void { + echo 'Fatal error listener called (should not happen)' . PHP_EOL; +}); + +$errorHandler = ErrorHandler::registerOnceExceptionHandler(); +$errorHandler->addExceptionHandlerListener(static function (): void { + echo 'Exception listener called' . PHP_EOL; +}); + +throw new \Exception('foo bar'); +--EXPECTF-- +Exception listener called + +Fatal error: Uncaught Exception: foo bar in %s:%d +Stack trace: +%a