Allow Bugsnag\Handler to handle OOMs

CHANGELOG.md

@@ -1,6 +1,13 @@
 Changelog
 =========
 
+## TBD
+
+### Enhancements
+
+* Out of memory errors will now be reported by increasing the memory limit by 5 MiB. Use the new `memoryLimitIncrease` configuration option to change the amount of memory, or set it to `null` to disable the increase entirely.
+  [#621](https://github.com/bugsnag/bugsnag-php/pull/621)
+
 ## 3.25.0 (2020-11-25)
 
 ### Enhancements

src/Client.php

@@ -935,4 +935,28 @@ public function getSessionClient()
     {
         return $this->config->getSessionClient();
     }
+
+    /**
+     * Set the amount to increase the memory_limit when an OOM is triggered.
+     *
+     * This is an amount of bytes or 'null' to disable increasing the limit.
+     *
+     * @param int|null $value
+     */
+    public function setMemoryLimitIncrease($value)
+    {
+        return $this->config->setMemoryLimitIncrease($value);
+    }
+
+    /**
+     * Get the amount to increase the memory_limit when an OOM is triggered.
+     *
+     * This will return 'null' if this feature is disabled.
+     *
+     * @return int|null
+     */
+    public function getMemoryLimitIncrease()
+    {
+        return $this->config->getMemoryLimitIncrease();
+    }
 }

src/Configuration.php

@@ -152,6 +152,15 @@ class Configuration
      */
     protected $buildEndpoint = self::BUILD_ENDPOINT;
 
+    /**
+     * The amount to increase the memory_limit to handle an OOM.
+     *
+     * The default is 5MiB and can be disabled by setting it to 'null'
+     *
+     * @var int|null
+     */
+    protected $memoryLimitIncrease = 5242880;
+
     /**
      * Create a new config instance.
      *
@@ -766,4 +775,30 @@ public function getSessionClient()
 
         return $this->sessionClient;
     }
+
+    /**
+     * Set the amount to increase the memory_limit when an OOM is triggered.
+     *
+     * This is an amount of bytes or 'null' to disable increasing the limit.
+     *
+     * @param int|null $value
+     */
+    public function setMemoryLimitIncrease($value)
+    {
+        $this->memoryLimitIncrease = $value;
+
+        return $this;
+    }
+
+    /**
+     * Get the amount to increase the memory_limit when an OOM is triggered.
+     *
+     * This will return 'null' if this feature is disabled.
+     *
+     * @return int|null
+     */
+    public function getMemoryLimitIncrease()
+    {
+        return $this->memoryLimitIncrease;
+    }
 }

src/Handler.php

@@ -28,6 +28,25 @@ class Handler
      */
     protected $previousExceptionHandler;
 
+    /**
+     * A bit of reserved memory to ensure we are able to increase the memory
+     * limit on an OOM.
+     *
+     * We can't reserve all of the memory that we need to send OOM reports
+     * because this would have a big overhead on every request, instead of just
+     * on shutdown in requests with errors.
+     *
+     * @var string|null
+     */
+    private $reservedMemory;
+
+    /**
+     * A regex that matches PHP OOM errors.
+     *
+     * @var string
+     */
+    private $oomRegex = '/^Allowed memory size of (\d+) bytes exhausted \(tried to allocate \d+ bytes\)/';
+
     /**
      * Whether the shutdown handler will run.
      *
@@ -136,6 +155,9 @@ public function registerExceptionHandler($callPrevious)
      */
     public function registerShutdownHandler()
     {
+        // Reserve some memory that we can free in the shutdown handler
+        $this->reservedMemory = str_repeat(' ', 1024 * 32);
+
         register_shutdown_function([$this, 'shutdownHandler']);
     }
 
@@ -267,6 +289,9 @@ public function errorHandler($errno, $errstr, $errfile = '', $errline = 0)
      */
     public function shutdownHandler()
     {
+        // Free the reserved memory to give ourselves some room to work
+        $this->reservedMemory = null;
+
         // If we're disabled, do nothing. This avoids reporting twice if the
         // exception handler is forcing the native PHP handler to run
         if (!self::$enableShutdownHandler) {
@@ -275,6 +300,17 @@ public function shutdownHandler()
 
         $lastError = error_get_last();
 
+        // If this is an OOM and memory increase is enabled, bump the memory
+        // limit so we can report it
+        if ($lastError !== null
+            && $this->client->getMemoryLimitIncrease() !== null
+            && preg_match($this->oomRegex, $lastError['message'], $matches) === 1
+        ) {
+            $currentMemoryLimit = (int) $matches[1];
+
+            ini_set('memory_limit', $currentMemoryLimit + $this->client->getMemoryLimitIncrease());
+        }
+
         // Check if a fatal error caused this shutdown
         if (!is_null($lastError) && ErrorTypes::isFatal($lastError['type']) && !$this->client->getConfig()->shouldIgnoreErrorCode($lastError['type'])) {
             $report = Report::fromPHPError(

tests/ClientTest.php

@@ -1127,6 +1127,25 @@ public function testMakeGuzzleCreatesTimeoutCanBeSpecified()
         $this->assertSame(2, $connectTimeout);
     }
 
+    public function testMemoryLimitIncreaseDefault()
+    {
+        $this->assertSame(1024 * 1024 * 5, $this->client->getMemoryLimitIncrease());
+    }
+
+    public function testMemoryLimitIncreaseCanBeSet()
+    {
+        $this->client->setMemoryLimitIncrease(12345);
+
+        $this->assertSame(12345, $this->client->getMemoryLimitIncrease());
+    }
+
+    public function testMemoryLimitIncreaseCanBeSetToNull()
+    {
+        $this->client->setMemoryLimitIncrease(null);
+
+        $this->assertNull($this->client->getMemoryLimitIncrease());
+    }
+
     private function getGuzzleOption($guzzle, $name)
     {
         if (GuzzleCompat::isUsingGuzzle5()) {

tests/ConfigurationTest.php

@@ -306,4 +306,23 @@ public function testTheSessionEndpointCanBeSetIfNecessary()
 
         $this->assertSame($expected, $this->config->getSessionEndpoint());
     }
+
+    public function testMemoryLimitIncreaseDefault()
+    {
+        $this->assertSame(1024 * 1024 * 5, $this->config->getMemoryLimitIncrease());
+    }
+
+    public function testMemoryLimitIncreaseCanBeSet()
+    {
+        $this->config->setMemoryLimitIncrease(12345);
+
+        $this->assertSame(12345, $this->config->getMemoryLimitIncrease());
+    }
+
+    public function testMemoryLimitIncreaseCanBeSetToNull()
+    {
+        $this->config->setMemoryLimitIncrease(null);
+
+        $this->assertNull($this->config->getMemoryLimitIncrease());
+    }
 }

tests/phpt/_prelude.php

@@ -6,6 +6,8 @@
 use Bugsnag\Configuration;
 use Bugsnag\Tests\phpt\Utilities\FakeGuzzle;
 
+date_default_timezone_set('UTC');
+
 $config = new Configuration('11111111111111111111111111111111');
 $config->setNotifyEndpoint('http://localhost/notify');
 $config->setSessionEndpoint('http://localhost/sessions');

tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt

@@ -0,0 +1,30 @@
+--TEST--
+Bugsnag\Handler should increase the memory limit by the configured amount when an OOM happens
+--FILE--
+<?php
+$client = require __DIR__ . '/_prelude.php';
+$client->setMemoryLimitIncrease(1024 * 1024 * 10);
+
+ini_set('memory_limit', '2M');
+var_dump(ini_get('memory_limit'));
+
+$client->registerCallback(function () {
+    var_dump(ini_get('memory_limit'));
+});
+
+Bugsnag\Handler::register($client);
+
+$a = str_repeat('a', 2147483647);
+
+echo "No OOM!\n";
+?>
+--EXPECTF--
+string(2) "2M"
+
+Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line 14
+string(8) "12582912"
+Guzzle request made (1 event)!
+* Method: 'POST'
+* URI: 'http://localhost/notify'
+* Events:
+    - Allowed memory size of %d bytes exhausted (tried to allocate %d bytes)

tests/phpt/handler_should_not_increase_memory_limit_when_memory_limit_increase_is_disabled.phpt

@@ -0,0 +1,32 @@
+--TEST--
+Bugsnag\Handler should not increase the memory limit when memoryLimitIncrease is disabled
+--FILE--
+<?php
+$client = require __DIR__ . '/_prelude.php';
+$client->setMemoryLimitIncrease(null);
+
+ini_set('memory_limit', '5M');
+var_dump(ini_get('memory_limit'));
+
+$client->registerCallback(function () {
+    // This should be the same as the first var_dump, because we should not have
+    // increase the memory limit
+    var_dump(ini_get('memory_limit'));
+});
+
+Bugsnag\Handler::register($client);
+
+$a = str_repeat('a', 2147483647);
+
+echo "No OOM!\n";
+?>
+--EXPECTF--
+string(2) "5M"
+
+Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line 16
+string(2) "5M"
+Guzzle request made (1 event)!
+* Method: 'POST'
+* URI: 'http://localhost/notify'
+* Events:
+    - Allowed memory size of %d bytes exhausted (tried to allocate %d bytes)

tests/phpt/handler_should_report_oom_from_large_allocation.phpt

@@ -0,0 +1,21 @@
+--TEST--
+Bugsnag\Handler should report OOMs triggered by single large allocations
+--FILE--
+<?php
+$client = require __DIR__ . '/_prelude.php';
+
+ini_set('memory_limit', '5M');
+
+Bugsnag\Handler::register($client);
+
+$a = str_repeat('a', 2147483647);
+
+echo "No OOM!\n";
+?>
+--EXPECTF--
+Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line 8
+Guzzle request made (1 event)!
+* Method: 'POST'
+* URI: 'http://localhost/notify'
+* Events:
+    - Allowed memory size of %d bytes exhausted (tried to allocate %d bytes)

tests/phpt/handler_should_report_oom_from_small_allocations.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Bugsnag\Handler should report OOMs triggered by many small allocations
+--FILE--
+<?php
+
+$client = require __DIR__ . '/_prelude.php';
+
+$handler = Bugsnag\Handler::register($client);
+
+ini_set('memory_limit', '5M');
+
+$i = 0;
+
+gc_disable();
+
+while ($i++ < 12345678) {
+    $a = new stdClass;
+    $a->b = $a;
+}
+
+echo "No OOM!\n";
+?>
+--SKIPIF--
+<?php
+if (PHP_MAJOR_VERSION < 7) {
+    echo "SKIP - PHP 5 does not run OOM in this test";
+}
+?>
+--EXPECTF--
+Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line %d
+Guzzle request made (1 event)!
+* Method: 'POST'
+* URI: 'http://localhost/notify'
+* Events:
+    - Allowed memory size of %d bytes exhausted (tried to allocate %d bytes)

tests/phpt/php5/handler_should_report_oom_from_small_allocations.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Bugsnag\Handler should report OOMs triggered by many small allocations
+--FILE--
+<?php
+
+$client = require __DIR__ . '/../_prelude.php';
+
+$handler = Bugsnag\Handler::register($client);
+
+ini_set('memory_limit', '5M');
+
+$size = 1024 * 256;
+
+$array = new SplFixedArray($size);
+
+for ($i = 0; $i < $size; ++$i) {
+    $array[$i] = str_repeat('a', 32);
+}
+
+echo "No OOM!\n";
+?>
+--SKIPIF--
+<?php
+if (PHP_MAJOR_VERSION !== 5) {
+    echo 'SKIP — this test does not run on PHP 7 & 8';
+}
+?>
+--EXPECTF--
+Fatal error: Allowed memory size of %d bytes exhausted (tried to allocate %d bytes) in %s on line 14
+Guzzle request made (1 event)!
+* Method: 'POST'
+* URI: 'http://localhost/notify'
+* Events:
+    - Allowed memory size of %d bytes exhausted (tried to allocate %d bytes)