php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #66442 Memory leaks after Interned String Pool exhaustion
Submitted: 2014-01-08 18:24 UTC Modified: 2014-01-10 18:49 UTC
From: Terry at ellisons dot org dot uk Assigned:
Status: Closed Package: opcache
PHP Version: master-Git-2014-01-08 (Git) OS: N/A
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: Terry at ellisons dot org dot uk
New email:
PHP Version: OS:

 

 [2014-01-08 18:24 UTC] Terry at ellisons dot org dot uk 
Description:
------------
The SHM Interned Strings pool is sized by the opcache.interned_strings_buffer INI parameter.  However, there is no guarantee that -- especially for large OPcaches supporting a lot of modules -- that the interned strings pool might not fill.  OPcache is then supposed to degrade gracefully into using non-interned memory for overflow strings that would otherwise have been allocated to SHM.

I decided to exercise these code paths within the tests environment by the following mod:

--- a/ZendAccelerator.c
+++ b/ZendAccelerator.c
@@ -2492,6 +2495,11 @@ static int zend_accel_init_shm(TSRMLS_D)
 #  if ZEND_EXTENSION_API_NO > PHP_5_5_X_API_NO
        orig_interned_empty_string = CG(interned_empty_string);
     CG(interned_empty_string) = accel_new_interned_string("", sizeof(""), 0 TSRMLS_CC);
+#   if 1            //TERRY
+    /* Q&D patch to force interned string pool exhaustion */
+       ZCSG(interned_strings_end) = CG(interned_strings_top)+128;
+       CG(interned_strings_end) = ZCSG(interned_strings_end);
+#   endif
 #  endif
 # endif

This set the strings end point to 128 above the top after initialising the SHM with existing interned strings.  This means in practice that the pool will "fill" after only a few string inserts and any remaining string will be handled by the "pool full" path.  

When I do this, I get a lot of memory leaks.  The following is a simple example:

Test script:
---------------
<?php
$c = get_declared_classes();
?>

run with the above patch in OPcache , with and without OPcache enabled.

Expected result:
----------------
The failover to non-SHM overflow should be silent.  No errors should be raised.  

Actual result:
--------------
$ php56 -d opcache.enable=0 /tmp/test.php
$ php56 -d opcache.enable=1 -d opcache.enable_cli=1 /tmp/test.php
[Wed Jan  8 18:21:11 2014]  Script:  '/tmp/test.php'
/.../Zend/zend_API.c(1461) :  Freeing 0x7F3162054110 (23 bytes), script=/tmp/test.php
Last leak repeated 41 times
=== Total 42 memory leaks detected ===

Footnote. This highlights that Sysadmins need to able to query capacity / usage information on all fixed size pools and tables.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-01-09 05:52 UTC] Terry at ellisons dot org dot uk 
I am still trying to track this down with not much success :-(  By setting a break at the "ZCSG(interned_strings_end) = CG(interned_strings_top)+128;"  line and jumping over these two (or not), I can exercise & trace out both code paths.  The difficultly is seeing where they've diverged.

Not that this leak is very sensitive to the script. E.g. get_loaded_extensions() doesn't create leaks.  Let me think about my test and instrumentation approach. I would to break the back of the analysis today  and your out the exact failure.
 [2014-01-09 05:54 UTC] Terry at ellisons dot org dot uk 
That should have read "Let me think about my test and instrumentation approach. I would like to break the back of the analysis today, and to identify the exact failure.
 [2014-01-09 07:31 UTC] Terry at ellisons dot org dot uk 
The Q&D patch is wrong -- I've replaced this with better one: replacing the "1024 * 1024" multipliers used with the interned_strings_buffer parameter to a simple 1024.  This makes it an nK parameter, and I can then set the  "opcache.interned_strings_buffer" to trigger pool-full code paths.  I get a different bug profile.   I'll update this later today with a proper analysis.
 [2014-01-09 18:17 UTC] Terry at ellisons dot org dot uk
-Status: Open +Status: Closed
 [2014-01-09 18:17 UTC] Terry at ellisons dot org dot uk 
OK, temporarily setting the opcache.interned_strings_buffer quantum to K units instead of M units achieved what I wanted.  I can now do a run-test.php across my php56 dev build cleanly -- 9102 passed / 3879 skipped and only 8 failed (the same as without this patch).  So I am now pretty confident that Interned String Pool exhaustion is handled gracefully without memory leaks. And this rep can be closed, after the small change suggested below.  That's good to know :)

The mimimum cache size must be big enough to do startup (e.g. execute php56 -r ''), and that depends on the extensions loaded.  For my test rig this is 177K.  Smaller than that and the ZE craters with memory corruption. However given that the default (4Mb > 177Kb) this shouldn't be an issue -- except that opcache.interned_strings_buffer=0 is allowed and this also causes the segment fault. Perhaps zend_accelerator_module.c should be modified to force a minium value of 1.

I do comment that its a pity that the only realistic way to test these code paths properly is to patch the code :(
 [2014-01-10 18:49 UTC] Terry at ellisons dot org dot uk 
I've raised a new bugrep (#66461) to patch the specific issue of the INI opcache.interned_strings_buffer=0, allowing this to be closed properly
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Fri Apr 19 03:01:27 2024 UTC