Skip to content

Commit

Permalink
Fix capturing OOM errors when very memory constrained (#1636)
Browse files Browse the repository at this point in the history
  • Loading branch information
stayallive authored Nov 13, 2023
1 parent 1988283 commit 33f6bf7
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 22 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion phpunit.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@
</php>

<testsuites>
<testsuite name="Sentry for PHP">
<testsuite name="unit">
<directory>tests</directory>
<directory suffix=".phpt">tests/phpt</directory>
</testsuite>

<testsuite name="oom">
<directory suffix=".phpt">tests/phpt-oom</directory>
</testsuite>
</testsuites>

<coverage>
Expand Down
63 changes: 56 additions & 7 deletions src/ErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 (?<memory_limit>\d+) bytes exhausted[^\r\n]* \(tried to allocate \d+ bytes\)/';

/**
* The fatal error types that cannot be silenced using the @ operator in PHP 8+.
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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, []);
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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--
<?php

declare(strict_types=1);

namespace Sentry\Tests;

use Sentry\ClientBuilder;
use Sentry\Event;
use Sentry\Options;
use Sentry\SentrySdk;
use Sentry\Serializer\PayloadSerializer;
use Sentry\Serializer\PayloadSerializerInterface;
use Sentry\Transport\Result;
use Sentry\Transport\ResultStatus;
use Sentry\Transport\TransportInterface;

$vendor = __DIR__;

while (!file_exists($vendor . '/vendor')) {
$vendor = \dirname($vendor);
}

require $vendor . '/vendor/autoload.php';

$options = new Options([
'dsn' => '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
22 changes: 10 additions & 12 deletions tests/phpt/error_handler_captures_out_of_memory_fatal_error.phpt
Original file line number Diff line number Diff line change
@@ -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--
<?php

Expand All @@ -19,23 +19,21 @@ while (!file_exists($vendor . '/vendor')) {

require $vendor . '/vendor/autoload.php';

$errorHandler = ErrorHandler::registerOnceErrorHandler();
$errorHandler->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
36 changes: 36 additions & 0 deletions tests/phpt/error_handler_handles_exception_only_once.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
Test that exceptions are only handled once
--FILE--
<?php

declare(strict_types=1);

namespace Sentry\Tests;

use Sentry\ErrorHandler;

$vendor = __DIR__;

while (!file_exists($vendor . '/vendor')) {
$vendor = \dirname($vendor);
}

require $vendor . '/vendor/autoload.php';

$errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
$errorHandler->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

0 comments on commit 33f6bf7

Please sign in to comment.