Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:70451
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.45 as permitted sender)
Message-ID: <52993190.4010905@gmail.com>
Date: Sat, 30 Nov 2013 00:30:08 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1
MIME-Version: 1.0
To: PHP Internals <internals@lists.php.net>
CC: Joe Watkins <krakjoe@php.net>, arjen@react.com, 
 Ferenc Kovacs <tyra3l@gmail.com>
References: <528CE64A.1020303@gmail.com> <CAH-PCH5RP1HRzC+qz94B862zrPt5O3x=U7MDvJJDPOJmAEkPjg@mail.gmail.com> <528EA006.2090400@gmail.com>
In-Reply-To: <528EA006.2090400@gmail.com>
Content-Type: multipart/alternative;
 boundary="------------010505090005070006040703"
Subject: Re: [PHP-DEV] Comments on non-unique naming convention for closures
From: ellison.terry@gmail.com (Terry Ellison)

--------------010505090005070006040703
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


>> 2013.11.20. 17:42, "Terry Ellison" <ellison.terry@gmail.com
>> <mailto:ellison.terry@gmail.com>> ezt írta:
>>  >
>>  > The following bugs relate to this discussion:
>>  >
>>  >     #64291 Indeterminate GC of evaled lambda function resources
>>  >     #65915 Inconsistent results with require return value
>>  >
>>  > I don't want to discuss these or the specific fixes here, since I can
>> work up a fix and discuss them in the bugrep.  However,
>>  > my one-sentence Q is "should we replace the naming convention for
>> closures with a truly unique one?"  What I would like is some feedback /
>> guidance / discussion on the general architectural issue which underlies
>> the reasons for these bugs occuring in the first place....
>>
>> Bumping the thread and ccing Joe as we were having a similar discussion
>> about the internal naming of the inner classes proposed by him.
>>
> Thanks for bumping this.
>
> Yup, the current build_runtime_defined_function_key() algo builds the 
> function name on the file name (closures) / the class/function name 
> and LANG_SCNG(yy_text) -- that is the memory address of the class / 
> function token being compiled. This is for closures, inner classes or 
> any class or function within a conditional block.
>
> I guess this is on the assumption that this will be unique, but as I 
> have shown in #64291 and #65915 it is fairly straight forward to 
> construct test cases where this assumption is invalid, and arjan seems 
> to be hitting this in real-life OPcache cases.
>
> My patch at least demonstrates that having a unique key will fix this, 
> but as submitted (adding a simple sequence) this will only reduces 
> this occurrence.  However strictly, this could still fail in OPcache 
> because of sequence duplication across forked PHP children.  We really 
> need a UUID-style element in the key say based on the creation 
> microtime (or alternatively start time of request + sequence within 
> request) and PID.
>
I have tabled a couple of PHPT tests on the bugreps which show this 
error consistently.

Having discussed the options with Dmitry and another contributor off PHP 
Internals list, we have decided to base the generated <internal> 
function name on a hash of the source content between the text pointers 
at zend_do_begin_function_declaration() and 
zend_do_end_function_declaration(). I will prepare a patch on this basis 
in the coming week.

Thanks and regards
Terry Ellison

--------------010505090005070006040703--