diff --git a/CHANGELOG.md b/CHANGELOG.md index a04b368d..18cfb28c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Client.php b/src/Client.php index 5d7be7de..b53f20cd 100644 --- a/src/Client.php +++ b/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(); + } } diff --git a/src/Configuration.php b/src/Configuration.php index d6d900a2..01784897 100644 --- a/src/Configuration.php +++ b/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; + } } diff --git a/src/Handler.php b/src/Handler.php index 8c2dee81..3dcc453f 100644 --- a/src/Handler.php +++ b/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( diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 0df26ae7..526d623b 100644 --- a/tests/ClientTest.php +++ b/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()) { diff --git a/tests/ConfigurationTest.php b/tests/ConfigurationTest.php index 808dddff..27bad4b7 100644 --- a/tests/ConfigurationTest.php +++ b/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()); + } } diff --git a/tests/phpt/_prelude.php b/tests/phpt/_prelude.php index c54f0a37..26cfd04a 100644 --- a/tests/phpt/_prelude.php +++ b/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'); diff --git a/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt b/tests/phpt/handler_should_increase_memory_limit_by_configured_amount_on_oom.phpt new file mode 100644 index 00000000..46ac7442 --- /dev/null +++ b/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-- +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) diff --git a/tests/phpt/handler_should_not_increase_memory_limit_when_memory_limit_increase_is_disabled.phpt b/tests/phpt/handler_should_not_increase_memory_limit_when_memory_limit_increase_is_disabled.phpt new file mode 100644 index 00000000..496ea60f --- /dev/null +++ b/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-- +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) diff --git a/tests/phpt/handler_should_report_oom_from_large_allocation.phpt b/tests/phpt/handler_should_report_oom_from_large_allocation.phpt new file mode 100644 index 00000000..e475c65c --- /dev/null +++ b/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-- + +--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) diff --git a/tests/phpt/handler_should_report_oom_from_small_allocations.phpt b/tests/phpt/handler_should_report_oom_from_small_allocations.phpt new file mode 100644 index 00000000..e7af2dda --- /dev/null +++ b/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-- +b = $a; +} + +echo "No OOM!\n"; +?> +--SKIPIF-- + +--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) diff --git a/tests/phpt/php5/handler_should_report_oom_from_small_allocations.phpt b/tests/phpt/php5/handler_should_report_oom_from_small_allocations.phpt new file mode 100644 index 00000000..eed45f34 --- /dev/null +++ b/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-- + +--SKIPIF-- + +--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)