Hi Nikita, I think the effect of the remaining patch is negligible. It disables things, that shouldn't work by design, didn't work in fact and leaded to crashes. It might work only in some cases and only because of luck.
So, I think it's better to commit this before next RC. Anatol is going to start rolling it on Tuesday. He will also increase all API version numbers. It would be great, if we stop any commits into PHP-7.0 except for critical fixes now Thanks. Dmitry. On Sat, Oct 10, 2015 at 4:26 PM, Nikita Popov <nikita....@gmail.com> wrote: > Hi internals! > > We have recently been reviewing the interaction between > ReflectionFunctionAbstract::getClosure(), a mechanism which converts an > ordinary function or method into a "fake" closure, and closure rebinding > using Closure::bindTo() and Closure::call(). > > It turns out that this combination has not yet received testing and > multiple crashes and leaks were found and fixed [1] [2] [3] [4]. > > We have one last outstanding changeset [5] waiting to land, which we want > to check back with internals first, as it constitutes a BC break late in > the PHP 7.0 release cycle. > > This changeset forbids rebinding the *scope* of closures returned by > getClosure() completely. The background of this change is that PHP 7 > includes optimizations that early-bind the scope of self:: during > compilation (and there are more optimizations of this nature pending to > land in PHP 7.1). This means that attempts to change the meaning of "self" > in an ordinary method at runtime will not always succeed and lead to > inconsistent results. > > Based on feedback received on an earlier version of this patch [6] [7], we > have not added any additional restrictions on rebinding of $this. > getClosure()s of userland methods can still be bound to arbitrary $this > values and getClosure()s of internal methods can still be bound to > "compatible" $this values. (If someone sees a problem with allowing the > former, please speak up!) > > Note that all this applies to ReflectionFunctionAbstract::getClosure() > only. Rebinding behavior of ordinary closures doesn't change. > > It would be appreciated if some more people familiar with the closure > implementation could take a look at our rebinding rules and see if there's > any situations that aren't covered yet and could be problematic. > > Thanks, > Nikita > > [1] https://bugs.php.net/bug.php?id=70630 > [2] https://bugs.php.net/bug.php?id=70674 > [3] https://bugs.php.net/bug.php?id=70681 > [4] https://bugs.php.net/bug.php?id=70685 > [5] https://github.com/php/php-src/pull/1560 > [6] > https://github.com/php/php-src/commit/517b5536259ecf7697f353f4bfbafde857fc1f81 > [7] > https://github.com/php/php-src/commit/881c50252066132f83e190325e344f532be19033 >
