fix(error): prevent warnings when trying to increase memory for OOM errors (#2063)

Author: Litarnus

src/ErrorHandler.php

@@ -394,8 +394,15 @@ private function handleFatalError(): void
                 && preg_match(self::OOM_MESSAGE_MATCHER, $error['message'], $matches) === 1
             ) {
                 $currentMemoryLimit = (int) $matches['memory_limit'];
+                $newMemoryLimit = $currentMemoryLimit + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue;
 
-                ini_set('memory_limit', (string) ($currentMemoryLimit + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue));
+                // It can happen that the memory limit + increase is still lower than
+                // the memory that is currently being used. This produces warnings
+                // that may end up in Sentry. To prevent this, we can check the real
+                // usage before.
+                if ($newMemoryLimit > memory_get_usage(true)) {
+                    $this->setMemoryLimitWithoutHandlingWarnings($newMemoryLimit);
+                }
 
                 self::$didIncreaseMemoryLimit = true;
             }
@@ -452,6 +459,23 @@ private function handleException(\Throwable $exception): void
         $this->handleException($previousExceptionHandlerException);
     }
 
+    /**
+     * Set the memory_limit while having no real error handler so that a warning emitted
+     * will not get reported.
+     */
+    private function setMemoryLimitWithoutHandlingWarnings(int $memoryLimit): void
+    {
+        set_error_handler(static function (): bool {
+            return true;
+        }, \E_WARNING);
+
+        try {
+            ini_set('memory_limit', (string) $memoryLimit);
+        } finally {
+            restore_error_handler();
+        }
+    }
+
     /**
      * Cleans and returns the backtrace without the first frames that belong to
      * this error handler.

tests/phpt/error_handler_does_not_capture_memory_limit_increase_warning_during_out_of_memory_handling.phpt

@@ -0,0 +1,73 @@
+--TEST--
+Test that OOM handling does not capture warnings from the memory limit increase attempt
+--INI--
+memory_limit=67108864
+--FILE--
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry {
+    // override the function so that we can trace how often it got invoked
+    function ini_set(string $option, string $value)
+    {
+        if (strtolower($option) !== 'memory_limit') {
+            return \ini_set($option, $value);
+        }
+
+        $GLOBALS['sentry_test_ini_set_calls'] = ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) + 1;
+
+        return \ini_set($option, $value);
+    }
+
+    // override the function so that we can test if the memory gets increased to a value
+    // that is lower than currently in use
+    function memory_get_usage(bool $realUsage = false): int
+    {
+        return $GLOBALS['sentry_test_memory_get_usage'] ?? \memory_get_usage($realUsage);
+    }
+}
+
+namespace Sentry\Tests {
+    use Sentry\ErrorHandler;
+
+    $vendor = __DIR__;
+
+    while (!file_exists($vendor . '/vendor')) {
+        $vendor = \dirname($vendor);
+    }
+
+    require $vendor . '/vendor/autoload.php';
+
+    error_reporting(\E_ALL & ~\E_DEPRECATED & ~\E_USER_DEPRECATED);
+
+    $GLOBALS['sentry_test_memory_get_usage'] = 1;
+
+    set_error_handler(static function (int $level): bool {
+        if (\E_WARNING !== $level) {
+            return false;
+        }
+
+        $GLOBALS['sentry_test_warning_handler_calls'] = ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) + 1;
+
+        return true;
+    });
+
+    $errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
+    $errorHandler->addFatalErrorHandlerListener(static function (): void {
+        echo 'Fatal error listener called' . \PHP_EOL;
+    });
+
+    register_shutdown_function(static function (): void {
+        echo 'Memory limit increase attempts: ' . ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) . \PHP_EOL;
+        echo 'Warning handler calls: ' . ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) . \PHP_EOL;
+    });
+
+    $foo = str_repeat('x', 1024 * 1024 * 1024);
+}
+?>
+--EXPECTF--
+%A
+Fatal error listener called
+Memory limit increase attempts: 1
+Warning handler calls: 0

tests/phpt/error_handler_skips_impossible_memory_limit_increase_during_out_of_memory_handling.phpt

@@ -0,0 +1,73 @@
+--TEST--
+Test that OOM handling skips the memory limit increase when current usage is already higher
+--INI--
+memory_limit=67108864
+--FILE--
+<?php
+
+declare(strict_types=1);
+
+namespace Sentry {
+    // override the function so that we can trace how often it got invoked
+    function ini_set(string $option, string $value)
+    {
+        if (strtolower($option) !== 'memory_limit') {
+            return \ini_set($option, $value);
+        }
+
+        $GLOBALS['sentry_test_ini_set_calls'] = ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) + 1;
+
+        return \ini_set($option, $value);
+    }
+
+    // override the function so that we can test if the memory gets increased to a value
+    // that is lower than currently in use
+    function memory_get_usage(bool $realUsage = false): int
+    {
+        return $GLOBALS['sentry_test_memory_get_usage'] ?? \memory_get_usage($realUsage);
+    }
+}
+
+namespace Sentry\Tests {
+    use Sentry\ErrorHandler;
+
+    $vendor = __DIR__;
+
+    while (!file_exists($vendor . '/vendor')) {
+        $vendor = \dirname($vendor);
+    }
+
+    require $vendor . '/vendor/autoload.php';
+
+    error_reporting(\E_ALL & ~\E_DEPRECATED & ~\E_USER_DEPRECATED);
+
+    $GLOBALS['sentry_test_memory_get_usage'] = (64 * 1024 * 1024) + (5 * 1024 * 1024) + 1;
+
+    set_error_handler(static function (int $level): bool {
+        if (\E_WARNING !== $level) {
+            return false;
+        }
+
+        $GLOBALS['sentry_test_warning_handler_calls'] = ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) + 1;
+
+        return true;
+    });
+
+    $errorHandler = ErrorHandler::registerOnceFatalErrorHandler();
+    $errorHandler->addFatalErrorHandlerListener(static function (): void {
+        echo 'Fatal error listener called' . \PHP_EOL;
+    });
+
+    register_shutdown_function(static function (): void {
+        echo 'Memory limit increase attempts: ' . ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) . \PHP_EOL;
+        echo 'Warning handler calls: ' . ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) . \PHP_EOL;
+    });
+
+    $foo = str_repeat('x', 1024 * 1024 * 1024);
+}
+?>
+--EXPECTF--
+%A
+Fatal error listener called
+Memory limit increase attempts: 0
+Warning handler calls: 0