Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:70269
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.212.178 as permitted sender)
Message-ID: <528EA006.2090400@gmail.com>
Date: Fri, 22 Nov 2013 00:06:30 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0
MIME-Version: 1.0
To: Ferenc Kovacs <tyra3l@gmail.com>
CC: PHP Internals <internals@lists.php.net>, 
 Joe Watkins <krakjoe@php.net>,
 arjen@react.com
References: <528CE64A.1020303@gmail.com> <CAH-PCH5RP1HRzC+qz94B862zrPt5O3x=U7MDvJJDPOJmAEkPjg@mail.gmail.com>
In-Reply-To: <CAH-PCH5RP1HRzC+qz94B862zrPt5O3x=U7MDvJJDPOJmAEkPjg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Comments on non-unique naming convention for closures
From: ellison.terry@gmail.com (Terry Ellison)

On 21/11/13 20:05, Ferenc Kovacs wrote:
>
> 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.

Regards
Terry