From aee4d218e603cfb684fd9ca6c9dc33ea62329cf0 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 22 Apr 2021 13:28:58 +0100 Subject: [PATCH 1/3] Avoid crash when bugsnag is not in the container This can happen when the DI container isn't totally setup, which is very common when running unit tests See https://github.com/bugsnag/bugsnag-laravel/issues/440 --- src/OomBootstrapper.php | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/OomBootstrapper.php b/src/OomBootstrapper.php index 1e7826db..6c8dd1c8 100644 --- a/src/OomBootstrapper.php +++ b/src/OomBootstrapper.php @@ -39,15 +39,26 @@ public function bootstrap() $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 - && app('bugsnag')->getMemoryLimitIncrease() !== null - && preg_match($this->oomRegex, $lastError['message'], $matches) === 1 - ) { + if (!$lastError) { + return; + } + + $isOom = preg_match($this->oomRegex, $lastError['message'], $matches) === 1; + + if (!$isOom) { + return; + } + + /** @var \Bugsnag\Client|null $client */ + $client = app('bugsnag'); + + // If the client exists and memory increase is enabled, bump the + // memory limit so we can report it. The client can be missing when + // the container isn't complete, e.g. when unit tests are running + if ($client && $client->getMemoryLimitIncrease() !== null) { $currentMemoryLimit = (int) $matches[1]; - ini_set('memory_limit', $currentMemoryLimit + app('bugsnag')->getMemoryLimitIncrease()); + ini_set('memory_limit', $currentMemoryLimit + $client->getMemoryLimitIncrease()); } }); } From d0d63110e3bc64325e48f65e6e57e8bc59b6d562 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 22 Apr 2021 13:41:20 +0100 Subject: [PATCH 2/3] Add some basic tests to check for crashes The OomBootstrapper is tested in our MazeRunner suite, so I've not replicated that here as it'd take a fair amount of setup to get running. Instead, these tests run the OomBootstrapper and will fail if it crashes --- .github/workflows/unit-tests.yml | 1 + .github/workflows/unstable-version-tests.yml | 1 + phpunit.xml.dist | 1 + ...uld_not_crash_when_bugsnag_is_missing.phpt | 47 +++++++++++++++ ...uld_not_crash_when_bugsnag_is_present.phpt | 59 +++++++++++++++++++ 5 files changed, 109 insertions(+) create mode 100644 tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt create mode 100644 tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 85f6ec19..d3d5af4f 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -106,6 +106,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} + coverage: none - run: composer validate diff --git a/.github/workflows/unstable-version-tests.yml b/.github/workflows/unstable-version-tests.yml index 459b2813..d86583fd 100644 --- a/.github/workflows/unstable-version-tests.yml +++ b/.github/workflows/unstable-version-tests.yml @@ -20,6 +20,7 @@ jobs: uses: shivammathur/setup-php@v2 with: php-version: ${{ matrix.php-version }} + coverage: none - run: composer validate diff --git a/phpunit.xml.dist b/phpunit.xml.dist index f42de868..e960dc8d 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,6 +18,7 @@ ./tests + ./tests/phpt diff --git a/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt new file mode 100644 index 00000000..e4a7c8e1 --- /dev/null +++ b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_missing.phpt @@ -0,0 +1,47 @@ +--TEST-- +The OomBootstrapper should not crash when bugsnag is not in the DI container +--FILE-- +bootstrap(); + +ini_set('memory_limit', '5M'); + +$i = 0; + +gc_disable(); + +while ($i++ < 12345678) { + $a = new stdClass; + $a->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 +'app' was called! diff --git a/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt new file mode 100644 index 00000000..3cfcaeb6 --- /dev/null +++ b/tests/phpt/oom_bootstrapper_should_not_crash_when_bugsnag_is_present.phpt @@ -0,0 +1,59 @@ +--TEST-- +The OomBootstrapper should not crash when bugsnag is in the DI container +--FILE-- +bootstrap(); + +ini_set('memory_limit', '5M'); + +$i = 0; + +gc_disable(); + +while ($i++ < 12345678) { + $a = new stdClass; + $a->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 +'app' was called! +'getMemoryLimitIncrease' was called! +'getMemoryLimitIncrease' was called! From 5c63d8adf7bde5183f262617c7c6a45b9460aba3 Mon Sep 17 00:00:00 2001 From: Joe Haines Date: Thu, 22 Apr 2021 14:50:49 +0100 Subject: [PATCH 3/3] Add a changelog entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b68830e1..6e68db50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ Changelog ========= +## TBD + +### Bug Fixes + +* Fix a possible crash in the OOM bootstrapper with an incomplete container + [#442](https://github.com/bugsnag/bugsnag-laravel/pull/442) + ## 2.22.0 (2021-02-10) ### Enhancements