Skip to content

Commit 372cf97

Browse files
LitarnusLitarnus
committed
fix(error): prevent warnings when trying to increase memory for OOM errors
1 parent 71c84eb commit 372cf97

3 files changed

Lines changed: 209 additions & 1 deletion

‎src/ErrorHandler.php‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,8 +394,15 @@ private function handleFatalError(): void
394394
&& preg_match(self::OOM_MESSAGE_MATCHER, $error['message'], $matches) === 1
395395
) {
396396
$currentMemoryLimit = (int) $matches['memory_limit'];
397+
$newMemoryLimit = $currentMemoryLimit + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue;
397398

398-
ini_set('memory_limit', (string) ($currentMemoryLimit + $this->memoryLimitIncreaseOnOutOfMemoryErrorValue));
399+
// It can happen that the memory limit + increase is still lower than
400+
// the memory that is currently being used. This produces warnings
401+
// that may end up in Sentry. To prevent this, we can check the real
402+
// usage before.
403+
if ($newMemoryLimit > memory_get_usage()) {
404+
$this->setMemoryLimitWithoutHandlingWarnings($newMemoryLimit);
405+
}
399406

400407
self::$didIncreaseMemoryLimit = true;
401408
}
@@ -452,6 +459,23 @@ private function handleException(\Throwable $exception): void
452459
$this->handleException($previousExceptionHandlerException);
453460
}
454461

462+
/**
463+
* Set the memory_limit while having no real error handler so that a warning emitted
464+
* will not get reported.
465+
*/
466+
private function setMemoryLimitWithoutHandlingWarnings(int $memoryLimit): void
467+
{
468+
set_error_handler(static function (): bool {
469+
return true;
470+
}, \E_WARNING);
471+
472+
try {
473+
ini_set('memory_limit', (string) $memoryLimit);
474+
} finally {
475+
restore_error_handler();
476+
}
477+
}
478+
455479
/**
456480
* Cleans and returns the backtrace without the first frames that belong to
457481
* this error handler.

‎tests/phpt/error_handler_does_not_capture_memory_limit_increase_warning_during_out_of_memory_handling.phpt‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
--TEST--
2+
Test that OOM handling does not capture warnings from the memory limit increase attempt
3+
--INI--
4+
memory_limit=67108864
5+
--FILE--
6+
<?php
7+
8+
declare(strict_types=1);
9+
10+
namespace Sentry {
11+
// override the function so that we can trace how often it got invoked
12+
function ini_set(string $option, string $value)
13+
{
14+
if (strtolower($option) !== 'memory_limit') {
15+
return \ini_set($option, $value);
16+
}
17+
18+
$GLOBALS['sentry_test_ini_set_calls'] = ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) + 1;
19+
20+
return \ini_set($option, $value);
21+
}
22+
23+
// override the function so that we can test if the memory gets increased to a value
24+
// that is lower than currently in use
25+
function memory_get_usage(bool $realUsage = false): int
26+
{
27+
return $GLOBALS['sentry_test_memory_get_usage'] ?? \memory_get_usage($realUsage);
28+
}
29+
}
30+
31+
namespace Sentry\Tests {
32+
use Sentry\Event;
33+
use Sentry\Transport\Result;
34+
use Sentry\Transport\ResultStatus;
35+
use Sentry\Transport\TransportInterface;
36+
37+
$vendor = __DIR__;
38+
39+
while (!file_exists($vendor . '/vendor')) {
40+
$vendor = \dirname($vendor);
41+
}
42+
43+
require $vendor . '/vendor/autoload.php';
44+
45+
error_reporting(\E_ALL & ~\E_DEPRECATED & ~\E_USER_DEPRECATED);
46+
47+
$GLOBALS['sentry_test_memory_get_usage'] = 1;
48+
49+
set_error_handler(static function (int $level): bool {
50+
if (\E_WARNING !== $level) {
51+
return false;
52+
}
53+
54+
$GLOBALS['sentry_test_warning_handler_calls'] = ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) + 1;
55+
56+
return true;
57+
});
58+
59+
$transport = new class implements TransportInterface {
60+
public function send(Event $event): Result
61+
{
62+
$GLOBALS['sentry_test_transport_calls'] = ($GLOBALS['sentry_test_transport_calls'] ?? 0) + 1;
63+
64+
return new Result(ResultStatus::success());
65+
}
66+
67+
public function close(?int $timeout = null): Result
68+
{
69+
return new Result(ResultStatus::success());
70+
}
71+
};
72+
73+
\Sentry\init([
74+
'dsn' => 'http://public@example.com/sentry/1',
75+
'transport' => $transport,
76+
'capture_silenced_errors' => true,
77+
]);
78+
79+
register_shutdown_function(static function (): void {
80+
echo 'Transport calls: ' . ($GLOBALS['sentry_test_transport_calls'] ?? 0) . \PHP_EOL;
81+
echo 'Memory limit increase attempts: ' . ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) . \PHP_EOL;
82+
echo 'Warning handler calls: ' . ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) . \PHP_EOL;
83+
});
84+
85+
$foo = str_repeat('x', 1024 * 1024 * 1024);
86+
}
87+
?>
88+
--EXPECTF--
89+
%A
90+
Transport calls: 1
91+
Memory limit increase attempts: 1
92+
Warning handler calls: 0

‎tests/phpt/error_handler_skips_impossible_memory_limit_increase_during_out_of_memory_handling.phpt‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
--TEST--
2+
Test that OOM handling skips the memory limit increase when current usage is already higher
3+
--INI--
4+
memory_limit=67108864
5+
--FILE--
6+
<?php
7+
8+
declare(strict_types=1);
9+
10+
namespace Sentry {
11+
// override the function so that we can trace how often it got invoked
12+
function ini_set(string $option, string $value)
13+
{
14+
if (strtolower($option) !== 'memory_limit') {
15+
return \ini_set($option, $value);
16+
}
17+
18+
$GLOBALS['sentry_test_ini_set_calls'] = ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) + 1;
19+
20+
return \ini_set($option, $value);
21+
}
22+
23+
// override the function so that we can test if the memory gets increased to a value
24+
// that is lower than currently in use
25+
function memory_get_usage(bool $realUsage = false): int
26+
{
27+
return $GLOBALS['sentry_test_memory_get_usage'] ?? \memory_get_usage($realUsage);
28+
}
29+
}
30+
31+
namespace Sentry\Tests {
32+
use Sentry\Event;
33+
use Sentry\Transport\Result;
34+
use Sentry\Transport\ResultStatus;
35+
use Sentry\Transport\TransportInterface;
36+
37+
$vendor = __DIR__;
38+
39+
while (!file_exists($vendor . '/vendor')) {
40+
$vendor = \dirname($vendor);
41+
}
42+
43+
require $vendor . '/vendor/autoload.php';
44+
45+
error_reporting(\E_ALL & ~\E_DEPRECATED & ~\E_USER_DEPRECATED);
46+
47+
$GLOBALS['sentry_test_memory_get_usage'] = (64 * 1024 * 1024) + (5 * 1024 * 1024) + 1;
48+
49+
set_error_handler(static function (int $level): bool {
50+
if (\E_WARNING !== $level) {
51+
return false;
52+
}
53+
54+
$GLOBALS['sentry_test_warning_handler_calls'] = ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) + 1;
55+
56+
return true;
57+
});
58+
59+
$transport = new class implements TransportInterface {
60+
public function send(Event $event): Result
61+
{
62+
$GLOBALS['sentry_test_transport_calls'] = ($GLOBALS['sentry_test_transport_calls'] ?? 0) + 1;
63+
64+
return new Result(ResultStatus::success());
65+
}
66+
67+
public function close(?int $timeout = null): Result
68+
{
69+
return new Result(ResultStatus::success());
70+
}
71+
};
72+
73+
\Sentry\init([
74+
'dsn' => 'http://public@example.com/sentry/1',
75+
'transport' => $transport,
76+
'capture_silenced_errors' => true,
77+
]);
78+
79+
register_shutdown_function(static function (): void {
80+
echo 'Transport calls: ' . ($GLOBALS['sentry_test_transport_calls'] ?? 0) . \PHP_EOL;
81+
echo 'Memory limit increase attempts: ' . ($GLOBALS['sentry_test_ini_set_calls'] ?? 0) . \PHP_EOL;
82+
echo 'Warning handler calls: ' . ($GLOBALS['sentry_test_warning_handler_calls'] ?? 0) . \PHP_EOL;
83+
});
84+
85+
$foo = str_repeat('x', 1024 * 1024 * 1024);
86+
}
87+
?>
88+
--EXPECTF--
89+
%A
90+
Transport calls: 1
91+
Memory limit increase attempts: 0
92+
Warning handler calls: 0

0 commit comments

Comments
 (0)