Bug #64291 [Com]: Indeterminate GC of evaled lambda function resources

Edit report at https://bugs.php.net/bug.php?id=64291&edit=1 ID: 64291 Comment by: Terry at ellisons dot org dot uk Reported by: Terry at ellisons dot org dot uk Summary: Indeterminate GC of evaled lambda function resources Status: Open Type: Bug Package: Scripting Engine problem Operating System: Ubuntu 12.10 PHP Version: 5.4.12 Block user comment: N Private report: N New Comment: Sorry, the above should read opcache.enable=0 though the failure is the same for opcache enabled and not enabled in the eval case. Previous Comments: ------------------------------------------------------------------------ [2013-11-25 13:24:08] Terry at ellisons dot org dot uk And here's the eval version: --TEST-- ISSUE #64291 Temporary function entries for closures are not unique --INI-- opcache.enable=1 --FILE-- <?php foreach (['a','b'] as $f) { $tmp = "function $f() {return function(){ return '$f'; };}\n"; eval($tmp); echo $tmp; } $a = a(); $b = b(); printf( "%s, %s\n ", $a(), $b()); ?> --EXPECT-- function a() {return function(){ return 'a'; };} function b() {return function(){ return 'b'; };} a, b ------------------------------------------------------------------------ [2013-11-25 12:42:16] Terry at ellisons dot org dot uk What threw me was the botch with the temporary entries "\0{closure}$filenane$offset" are used in the EG(function_table). This is as clear as mud. When a file is compiled, a function table entry is created for each closue in the source. This entry is never executed directly, but is used by the ZEND_DECLARE_LAMBDA_FUNCTION to construct the closure object which contains a deep copy of this zend_function record. It is this copy that used when the closure is called. So long as the ZEND_DECLARE_LAMBDA_FUNCTION are executed within the same scope as the compile, this should normally be unique, but it is quite easy to construct a test case which the unique assumption fails: --TEST-- ISSUE #65915 Temporary function entries for closures are not unique --INI-- opcache.enable=0 --SKIPIF-- --FILE-- <?php $tmp = tempnam(__DIR__, 'test'); foreach (['a','b'] as $f) { file_put_contents($tmp, "<?php function $f() {return function(){ return '$f'; };}"); echo file_get_contents($tmp), "\n"; require $tmp; } $a = a(); $b = b(); printf( "%s, %s\n ", $a(), $b()); unlink($tmp); ?> --CLEAN-- --EXPECT-- <?php function a() {return function(){ return 'a'; };} <?php function b() {return function(){ return 'b'; };} a, b ------------------------------------------------------------------------ [2013-11-25 08:53:35] dmitry@php.net Your second script prints "a, a" only with OPCache, because it caches the included temporary file. Without OPCache it prints the expected "a, b". I also don't think that the first script indicates a bug. The more functions you create the more memory it requires. ------------------------------------------------------------------------ [2013-11-22 12:34:08] Terry at ellisons dot org dot uk Just to note that my patch is not strong enough when used with OPcache, since a simple static running count can fail when the interpreter is forked. Better alternatives include * some form of unique id, e.g. a UUID or uniquid(true) * a content based hash, such as md5file(__FILE__) -- though if this were to be adopted then it would be better always to generate this as part of the compile creating say __FILE_MD5__, though this would add a few % to the compile time. This all needs wider discussion of these issues on the Internals ML. ------------------------------------------------------------------------ [2013-11-21 15:07:43] Terry at ellisons dot org dot uk The issue here is that the runtime defined function keys should be unique, and the current algo does not generate unique keys. There are many approaches that could be taken to remove such incorrect name clashes. The patch that I've submitted add a sequence count to the key -- simple but sufficient to prevent this bug. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=64291 -- Edit this bug report at https://bugs.php.net/bug.php?id=64291&edit=1