Fix capturing OOM errors when very memory constrained

.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

phpunit.xml.dist

@@ -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>

src/ErrorHandler.php

@@ -22,7 +22,14 @@
      *
      * @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+.
@@ -89,11 +96,27 @@
     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 @@
         $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 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 @@
         // 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;
         }

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--
+<?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

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--
 <?php
 
@@ -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

tests/phpt/error_handler_handles_exception_only_once.phpt

@@ -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