Forráskód Böngészése

Add method in Utils to convert memory values from php_ini into bytes, and use lower amount of chunk size based on memory limit

jcm 4 éve
szülő
commit
0b22036ab6

+ 33 - 4
src/Monolog/Handler/StreamHandler.php

@@ -25,8 +25,12 @@ use Monolog\Utils;
  */
  */
 class StreamHandler extends AbstractProcessingHandler
 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 */
     /** @var resource|null */
     protected $stream;
     protected $stream;
     /** @var ?string */
     /** @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)
     public function __construct($stream, $level = Logger::DEBUG, bool $bubble = true, ?int $filePermission = null, bool $useLocking = false)
     {
     {
         parent::__construct($level, $bubble);
         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)) {
         if (is_resource($stream)) {
             $this->stream = $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)) {
         } elseif (is_string($stream)) {
             $this->url = Utils::canonicalizePath($stream);
             $this->url = Utils::canonicalizePath($stream);
         } else {
         } else {
@@ -95,6 +116,14 @@ class StreamHandler extends AbstractProcessingHandler
         return $this->url;
         return $this->url;
     }
     }
 
 
+    /**
+     * @return int
+     */
+    public function getStreamChunkSize() : int
+    {
+        return $this->streamChunkSize;
+    }
+
     /**
     /**
      * {@inheritDoc}
      * {@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));
                 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;
             $this->stream = $stream;
         }
         }
 
 

+ 48 - 0
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;
+    }
 }
 }

+ 87 - 0
tests/Monolog/Handler/StreamHandlerTest.php

@@ -11,6 +11,7 @@
 
 
 namespace Monolog\Handler;
 namespace Monolog\Handler;
 
 
+use Monolog\Handler\StreamHandler;
 use Monolog\Test\TestCase;
 use Monolog\Test\TestCase;
 use Monolog\Logger;
 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);
+    }
 }
 }

+ 43 - 0
tests/Monolog/UtilsTest.php

@@ -141,4 +141,47 @@ class UtilsTest extends \PHPUnit_Framework_TestCase
             [-1, 'UNDEFINED_ERROR'],
             [-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);
+    }
 }
 }