From 0b22036ab67fa7752e53e8bb18891894e31137d1 Mon Sep 17 00:00:00 2001 From: jcm Date: Fri, 30 Jul 2021 09:06:22 +0200 Subject: [PATCH] Add method in Utils to convert memory values from php_ini into bytes, and use lower amount of chunk size based on memory limit --- src/Monolog/Handler/StreamHandler.php | 37 ++++++++- src/Monolog/Utils.php | 48 ++++++++++++ tests/Monolog/Handler/StreamHandlerTest.php | 87 +++++++++++++++++++++ tests/Monolog/UtilsTest.php | 43 ++++++++++ 4 files changed, 211 insertions(+), 4 deletions(-) diff --git a/src/Monolog/Handler/StreamHandler.php b/src/Monolog/Handler/StreamHandler.php index bc1f198d..8d118928 100644 --- a/src/Monolog/Handler/StreamHandler.php +++ b/src/Monolog/Handler/StreamHandler.php @@ -25,8 +25,12 @@ use Monolog\Utils; */ class StreamHandler extends AbstractProcessingHandler { - protected const MAX_CHUNK_SIZE = 2147483647; - + /** @const int */ + const SAFE_MEMORY_OFFSET = 1024; + /** @const int */ + const MAX_CHUNK_SIZE = 2147483647; + /** @var int */ + protected $streamChunkSize = self::MAX_CHUNK_SIZE; /** @var resource|null */ protected $stream; /** @var ?string */ @@ -50,9 +54,26 @@ class StreamHandler extends AbstractProcessingHandler public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false) { parent::__construct($level, $bubble); + + if ($phpMemoryLimit = ini_get('memory_limit')) { + if (($memoryInByes = Utils::memoryIniValueToBytes($phpMemoryLimit))) { + $memoryUsage = memory_get_usage(true); + if (($memoryInByes - $memoryUsage) < $this->streamChunkSize) { + $this->streamChunkSize = $memoryInByes - $memoryUsage - self::SAFE_MEMORY_OFFSET; + } + } + } + if (is_resource($stream)) { $this->stream = $stream; - stream_set_chunk_size($this->stream, self::MAX_CHUNK_SIZE); + + try { + stream_set_chunk_size($this->stream, $this->streamChunkSize); + } catch (\Exception $exception) { + throw new \RuntimeException('Impossible to set the stream chunk size.' + .PHP_EOL.'Error: '.$exception->getMessage() + .PHP_EOL.'Trace: '.$exception->getTraceAsString()); + } } elseif (is_string($stream)) { $this->url = Utils::canonicalizePath($stream); } else { @@ -95,6 +116,14 @@ class StreamHandler extends AbstractProcessingHandler return $this->url; } + /** + * @return int + */ + public function getStreamChunkSize() : int + { + return $this->streamChunkSize; + } + /** * {@inheritDoc} */ @@ -118,7 +147,7 @@ class StreamHandler extends AbstractProcessingHandler throw new \UnexpectedValueException(sprintf('The stream or file "%s" could not be opened in append mode: '.$this->errorMessage, $url)); } - stream_set_chunk_size($stream, self::MAX_CHUNK_SIZE); + stream_set_chunk_size($stream, $this->streamChunkSize); $this->stream = $stream; } diff --git a/src/Monolog/Utils.php b/src/Monolog/Utils.php index 8812f007..28a7f334 100644 --- a/src/Monolog/Utils.php +++ b/src/Monolog/Utils.php @@ -226,4 +226,52 @@ final class Utils ); } } + + /** + * Converts a string with a valid 'memory_limit' format, to bytes. + * Reference: Function code from https://www.php.net/manual/en/function.ini-get.php + * @param string|int $val + * @return int|false Returns an integer representing bytes. Returns FALSE in case of error. + */ + public static function memoryIniValueToBytes($val) + { + if (!is_string($val) && !is_integer($val)) { + return false; + } + + $val = trim((string)$val); + + if (empty($val)) { + return false; + } + + $valLen = strlen($val); + $last = strtolower($val[$valLen - 1]); + + if (preg_match('/[a-zA-Z]/', $last)) { + if ($valLen == 1) { + return false; + } + + $val = substr($val, 0, -1); + } + + if (!is_numeric($val) || $val < 0) { + return false; + } + + //Lets be explicit here + $val = (int)($val); + + switch ($last) { + case 'g': + $val *= 1024; + case 'm': + $val *= 1024; + case 'k': + $val *= 1024; + } + + return $val; + } } diff --git a/tests/Monolog/Handler/StreamHandlerTest.php b/tests/Monolog/Handler/StreamHandlerTest.php index 8e0d111f..8a9ae184 100644 --- a/tests/Monolog/Handler/StreamHandlerTest.php +++ b/tests/Monolog/Handler/StreamHandlerTest.php @@ -11,6 +11,7 @@ namespace Monolog\Handler; +use Monolog\Handler\StreamHandler; use Monolog\Test\TestCase; use Monolog\Logger; @@ -220,4 +221,90 @@ class StreamHandlerTest extends TestCase ], ]; } + + public function provideMemoryValues() + { + return [ + ['1M', true], + ['10M', true], + ['1024M', true], + ['1G', true], + ['2000M', true], + ['2050M', true], + ['2048M', true], + ['3G', false], + ['2560M', false], + ]; + } + + /** + * @dataProvider provideMemoryValues + * @return void + */ + public function testPreventOOMError($phpMemory, $chunkSizeDecreased) + { + $memoryLimit = ini_set('memory_limit', $phpMemory); + + if ($memoryLimit === false) { + /* + * We could not set a memory limit that would trigger the error. + * There is no need to continue with this test. + */ + + $this->assertTrue(true); + return; + } + + $stream = tmpfile(); + + if ($stream === false) { + /* + * We could create a temp file to be use as a stream. + * There is no need to continue with this test. + */ + $this->assertTrue(true); + return; + } + + $exceptionRaised = false; + + try { + $handler = new StreamHandler($stream); + stream_get_contents($stream, 1024); + } catch (\RuntimeException $exception) { + /* + * At this point, stream_set_chunk_size() failed in the constructor. + * Probably because not enough memory. + * The rest of th test depends on the success pf stream_set_chunk_size(), that is why + * if exception is raised (which is true at this point), the rest of assertions will not be executed. + */ + $exceptionRaised = true; + } + + ini_set('memory_limit', $memoryLimit); + $this->assertEquals($memoryLimit, ini_get('memory_limit')); + + if (!$exceptionRaised) { + $this->assertEquals($chunkSizeDecreased, $handler->getStreamChunkSize() < StreamHandler::MAX_CHUNK_SIZE); + } + } + + /** + * @return void + */ + public function testSimpleOOMPrevention() + { + $previousValue = ini_set('memory_limit', '2048M'); + + if ($previousValue === false) { + $this->assertTrue(true); + return; + } + + $stream = tmpfile(); + new StreamHandler($stream); + stream_get_contents($stream); + ini_set('memory_limit', $previousValue); + $this->assertTrue(true); + } } diff --git a/tests/Monolog/UtilsTest.php b/tests/Monolog/UtilsTest.php index bc216467..95d11fa5 100644 --- a/tests/Monolog/UtilsTest.php +++ b/tests/Monolog/UtilsTest.php @@ -141,4 +141,47 @@ class UtilsTest extends \PHPUnit_Framework_TestCase [-1, 'UNDEFINED_ERROR'], ]; } + + public function provideMemoryIniValuesToConvertToBytes() + { + return [ + ['1', 1], + ['2', 2], + ['2.5', 2], + ['2.9', 2], + ['1B', 1], + ['1X', 1], + ['1K', 1024], + ['1G', 1073741824], + ['', false], + [null, false], + ['A', false], + ['AA', false], + ['B', false], + ['BB', false], + ['G', false], + ['GG', false], + ['-1', false], + ['-123', false], + ['-1A', false], + ['-1B', false], + ['-123G', false], + ['-B', false], + ['-A', false], + ['-', false], + [true, false], + [false, false], + ]; + } + + /** + * @dataProvider provideMemoryIniValuesToConvertToBytes + * @param mixed $input + * @param int|false $expected + */ + public function testMemoryIniValueToBytes($input, $expected) + { + $result = Utils::memoryIniValueToBytes($input); + $this->assertEquals($expected, $result); + } }