Merge pull request #442 from bugsnag/fix-crash-in-oom-bootstrapper

Fix possible crash in the OOM bootstrapper

.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
 

.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
 

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

phpunit.xml.dist

@@ -18,6 +18,7 @@
     <testsuites>
         <testsuite name="Bugsnag Laravel Test Suite">
             <directory suffix="Test.php">./tests</directory>
+            <directory suffix=".phpt">./tests/phpt</directory>
         </testsuite>
     </testsuites>
 

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());
             }
         });
     }

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--
+<?php
+
+use Bugsnag\BugsnagLaravel\OomBootstrapper;
+
+function app($alias = null) {
+    echo "'app' was called!\n";
+
+    if ($alias === 'bugsnag') {
+        return null;
+    }
+
+    if ($alias !== null) {
+        throw new UnexpectedValueException("Unknown alias '{$alias}' given");
+    }
+
+    throw new BadFunctionCallException("This fake 'app' should always be called with an alias");
+}
+
+require __DIR__.'/../../src/OomBootstrapper.php';
+
+(new OomBootstrapper())->bootstrap();
+
+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
+'app' was called!

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--
+<?php
+
+use Bugsnag\BugsnagLaravel\OomBootstrapper;
+
+class FakeClient
+{
+    public function getMemoryLimitIncrease()
+    {
+        echo "'getMemoryLimitIncrease' was called!\n";
+
+        return 12345;
+    }
+}
+
+function app($alias = null) {
+    echo "'app' was called!\n";
+
+    if ($alias === 'bugsnag') {
+        return new FakeClient();
+    }
+
+    if ($alias !== null) {
+        throw new UnexpectedValueException("Unknown alias '{$alias}' given");
+    }
+
+    throw new BadFunctionCallException("This fake 'app' should always be called with an alias");
+}
+
+require __DIR__.'/../../src/OomBootstrapper.php';
+
+(new OomBootstrapper())->bootstrap();
+
+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
+'app' was called!
+'getMemoryLimitIncrease' was called!
+'getMemoryLimitIncrease' was called!