php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #65338 Enabling both php_opcache and php_wincache AVs on shutdown
Submitted: 2013-07-25 17:30 UTC Modified: 2013-07-30 13:39 UTC
Votes:1
Avg. Score:3.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:0 (0.0%)
From: ericsten@php.net Assigned: dmitry
Status: Closed Package: Reproducible crash
PHP Version: 5.5.1 OS: Windows
Private report: No CVE-ID:
 [2013-07-25 17:30 UTC] ericsten@php.net
Description:
------------
If both php_wincache.dll and php_opcache.dll are enabled, and if they are both enabled for CLI, any script leads to an AV at process exit.

The call stack indicates that the AV is in zend_interned_strings_dtor, on the following line:

          free(CG(interned_strings_start));

This is because the value in CG(interned_strings_start) is pointing at the chunk of memory provided by php_wincache.dll for its interned strings.  

I'm seeing in the debugger that on process startup, the modules are loaded in the order:

1.	php_wincache.dll
2.	php_opcache.dll

And on shutdown, they're terminated in exactly the same order.

This causes a problem, because both modules set the CG(interned_strings_start) based upon the value it copied during startup.  In this case, php_opcache.dll copied the value that php_wincache.dll set when it started up.  So, the last value put back into CG(interned_strings_start) on shutdown was php_wincache's interned strings buffer.  

php_wincache.dll allocated the interned strings block using (zend's) pemalloc(), but the address for CG(interned_strings_start) is an offset within the allocation, so free() thinks the heap is corrupted.

Question: 

Why are modules terminated in the same order they were initialized?  For modules that do 'hooking' of functions or memory, it seems the "unhooking" should happen in reverse order.

php.ini settings:

zend_extension=php_opcache.dll
extension=php_wincache.dll
[opcache]
opcache.enable=1
opcache.enable_cli=1
[wincache]
wincache.enablecli=1
wincache.ocenabled=0


Test script:
---------------
<?php
$variable = 2.0;
function testGlobal()
{
    global $variable;
    var_dump($variable);
}
testGlobal();
$variable += 1;
testGlobal();
$variable = "Changing to string.";
testGlobal();
?>

Expected result:
----------------
No AV on shutdown


Patches

zend_interned_strings_shutdown_AV (last revision 2013-07-25 17:30 UTC) by ericsten@php.net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-07-25 17:30 UTC] ericsten@php.net
The following patch has been added/updated:

Patch Name: zend_interned_strings_shutdown_AV
Revision:   1374773456
URL:        https://bugs.php.net/patch-display.php?bug=65338&patch=zend_interned_strings_shutdown_AV&revision=1374773456
 [2013-07-26 10:58 UTC] ab@php.net
-Status: Open +Status: Feedback
 [2013-07-26 10:58 UTC] ab@php.net
What is the catch/sense of using both at the same time? Both  are opcaches and can 
cross each other in many other hooks, most important replacing zend_compile_file.

It's beyond what happens, wincache replaces zend_compile_file with its own, followed by 
opcache replacing it with its own. Or vice versa. What happens to user trying to use 
the first loaded module then?
 [2013-07-26 16:04 UTC] ericsten@php.net
ab@php.net said
> What is the catch/sense of using both at the same time?

Wincache provides a file cache, session cache, user property cache as well as an opcode cache.  Further, it's possible to disable the opcode cache.

On PHP5.5, we (Wincache support folks) expect customers to enable the Zend opcache (because it's in 'core', and probably does more optimizing than Wincache does), but continue to use Wincache for file, session and user cache.
 [2013-07-26 16:48 UTC] phpdev at ehrhardt dot nl
@Anatol: Xinchen ran into the same problem while combining APC usercache with OPcache opcode cache. See
http://svn.php.net/viewvc?view=revision&revision=330859
He solved it by disabling interned_string.

@Eric: I can confirm your patch works. Tested it as well under X86 as X64.
 [2013-07-26 21:07 UTC] me at laurinkeithdavis dot com
I can confirm that this patch works as well.

https://bugs.php.net/bug.php?id=65247
 [2013-07-27 01:27 UTC] phpdev at ehrhardt dot nl
The problem is so obvious, that I am surprised it did not com to surface
earlier. And the patch is elegant: do not assume interned_strings_start
is still the same, but free only the memory that you owned at startup.

In fact, the patch should be backported to PHP 5.4 as well. I do not
have a use case for X86, but I ran into the same problem woth PHP 5.4
X64. I know this is no official version, but as an illustration of the
problem it still is useful.

Compare these two builds:
https://dl.dropboxusercontent.com/u/8954372/php-5.4.17-nts-Win32-VC9-x64.zip
https://dl.dropboxusercontent.com/u/8954372/php-5.4.17-nts-Win32-VC9-x64_patched.zip

Try the unpatched one first. Put this in your php.ini:

extension=php_wincache.dll
zend_extension=ext/php_opcache.dll

zend_opcache.memory_consumption=128
zend_opcache.interned_strings_buffer=8
zend_opcache.max_accelerated_files=4000
zend_opcache.revalidate_freq=60
zend_opcache.fast_shutdown=1
zend_opcache.enable_cli=1

wincache.ocenabled=0
wincache.fcenabled=0	
wincache.ucachesize=768
wincache.enablecli=1
session.savehandler=wincache

Then run from the commandline in your php-directory:
php-cgi.exe -m

php-cgi will crash after showing the loaded modules. Debugging with VC9
gave this result: http://x32.elijst.nl/zendfree.png
Quite another breakpoint as in the PHP 5.5 example, but with the same
cause: freeing memory you do not own.

In the patched build I backported Eric's patch for zend_string.c to PHP
5.4. Result: no crash anymore.

A last remark: i do not think the problem is Windows specific. This is
exectly the same problem, but with the combination of opcache and apc:
http://svn.php.net/viewvc?view=revision&revision=330859
 [2013-07-27 05:02 UTC] pajoye@php.net
-Assigned To: +Assigned To: dmitry
 [2013-07-27 05:02 UTC] pajoye@php.net
Agreed,should be applied in php-src and 
github repos.
 [2013-07-27 08:30 UTC] ab@php.net
@Eric ok, so the catch is to use the wincache with disabled opcode cache. Whereby 
interned strings are neither opcode cache nor any other functionality. The  patch 
would solve it only for newer PHP versions, maybe it should be possible to disable 
interned strings explicitly in wincache/opcache, maybe per ini? That would solve 
it also for users with older PHP without TS.

@Jan that's obvious now where people start to mix the cache engines :)
 [2013-07-27 12:18 UTC] phpdev at ehrhardt dot nl
@Anatol: there is no need to disable interned strings in extensions for older PHP-versions if you backport the patch. I do not think I have the karma to put a patch here, but this is the backport for PHP 5.4:
http://x32.elijst.nl/zend_interned_strings_shutdown_AV.patch

Maybe except for some line numbers it is literally the same patch.
 [2013-07-27 15:25 UTC] ab@php.net
Jan, I meant exactly that, there still will be some users on versions lower than 
5.4.17 or which is the current. The question is just whether it's worth the effort 
to be aware of that.
 [2013-07-27 16:56 UTC] phpdev at ehrhardt dot nl
@Anatol: which older versions? PHP 5.4.x users that run into this problem should upgrade to 5.4.18, the moment that is released with this patch. Making special arrangements for older 5.4.x users would be a waste of developer resources IMO.

And PHP 5.3 (or lower) users do not run into this, because interned_string was introduced in the Zend engine for 5.4.
 [2013-07-29 18:30 UTC] ericsten@php.net
@ab@php.net

Wincache.ocenabled is PHP_INI_ALL, meaning that the Wincache opcode cache can be enabled/disabled by script or by user.ini.  This is the same for Zend Opcache opcache.enabled and APC apc.enabled.  To support this, the interned strings have to be hooked at module load time.  This is how all three opcode caches implement the hooking of interned strings.
 [2013-07-30 07:06 UTC] ab@php.net
@Eric, yes, one can only enable/disable it in whole. What i meant is making it 
more 
flexible, disabling opcache and string pool, but not affecting other features 
like 
user cache.

Like APCu, but there opcode cache was just cut out. Splitting the functionality 
into 
configurable pieces would require some effort of course. And that was actually 
the 
idea you reflected at the earlier post. As neither opcode cache nor interned 
strings 
aren't required for user cache.
 [2013-07-30 13:36 UTC] dmitry@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=3550f3d0aad6e979e2a6fe3ee40d4fbff168c34b
Log: Fixed bug #65338 (Enabling both php_opcache and php_wincache AVs on shutdown).
 [2013-07-30 13:36 UTC] dmitry@php.net
-Status: Feedback +Status: Closed
 [2013-07-30 13:39 UTC] dmitry@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2013-07-30 15:13 UTC] phpdev at ehrhardt dot nl
@dmitry: grabbed the snapshot from http://git.php.net/?p=php-src.git;a=summary and compiled it. No crash anymore!

Nitpicking: typo OPcahce in the NEWS diff.

And curious: why did you decide to fix the problem by changing OPcache? Is not there a fundamental problem in zend_string.c that it sometimes frees memory while interned_strings_start has been changed? Or am I completely missing the point?
 [2013-11-17 09:30 UTC] laruence@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=3550f3d0aad6e979e2a6fe3ee40d4fbff168c34b
Log: Fixed bug #65338 (Enabling both php_opcache and php_wincache AVs on shutdown).
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Sun Apr 20 20:02:01 2014 UTC