Re: [PR 614] ArrayObject and isset() / empty() behaviour

Hi Tjerk, I was playing with ZF2 and getting segfaults for Mvc tests (for example phpunit ZendTest/Mvc/ApplicationTest.php). It's only on 5.6 and master branch. Think that it might be because of this patch: 0 zend_call_method zend_interfaces.c 35 0xae17fd 1 spl_array_read_dimension_ex spl_array.c 389 0x87179b 2 spl_array_has_dimension_ex spl_array.c 655 0x87273d 3 spl_array_has_dimension spl_array.c 675 0x8727d1 4 zend_isset_isempty_dim_prop_obj_handler_SPEC_CV_CV zend_vm_execute.h 40983 0xb817a8 5 ZEND_ISSET_ISEMPTY_DIM_OBJ_SPEC_CV_CV_HANDLER zend_vm_execute.h 41039 0xb819c1 6 execute_ex zend_vm_execute.h 363 0xb097cb 7 zend_execute zend_vm_execute.h 388 0xb098b1 8 zend_call_function zend_execute_API.c 952 0xaa6faa 9 zend_call_method zend_interfaces.c 97 0xae1bea 10 spl_array_read_dimension_ex spl_array.c 389 0x87179b 11 spl_array_has_dimension_ex spl_array.c 655 0x87273d 12 spl_array_has_dimension spl_array.c 675 0x8727d1 13 zend_isset_isempty_dim_prop_obj_handler_SPEC_CV_CV zend_vm_execute.h 40983 0xb817a8 14 ZEND_ISSET_ISEMPTY_DIM_OBJ_SPEC_CV_CV_HANDLER zend_vm_execute.h 41039 0xb819c1 15 execute_ex zend_vm_execute.h 363 0xb097cb 16 zend_execute zend_vm_execute.h 388 0xb098b1 17 zend_call_function zend_execute_API.c 952 0xaa6faa 18 zend_call_method zend_interfaces.c 97 0xae1bea 19 spl_array_read_dimension_ex spl_array.c 389 0x87179b 20 spl_array_has_dimension_ex spl_array.c 655 0x87273d ... <More> Let me know if you can't recreate it Thanks Jakub On Mon, Mar 10, 2014 at 4:20 PM, Tjerk Meesters <tjerk.meesters@gmail.com>wrote: > On Mon, Mar 10, 2014 at 9:58 PM, Tjerk Meesters <tjerk.meesters@gmail.com > >wrote: > > > Hi Etienne, > > > > > > On Mon, Mar 10, 2014 at 9:26 PM, Etienne Kneuss <colder@php.net> wrote: > > > >> > >> > >> > >> On Mon, Mar 10, 2014 at 1:43 PM, Ferenc Kovacs <tyra3l@gmail.com> > wrote: > >> > >>> > >>> > >>> > >>> On Sat, Mar 8, 2014 at 10:56 AM, Tjerk Meesters < > >>> tjerk.meesters@gmail.com> wrote: > >>> > >>>> On Sat, Mar 8, 2014 at 8:27 AM, Tjerk Meesters < > >>>> tjerk.meesters@gmail.com>wrote: > >>>> > >>>> > Hi Etienne, > >>>> > > >>>> > Thanks for your input, much appreciated. > >>>> > > >>>> > I have moved the decision up the chain as you suggested; the results > >>>> can > >>>> > be found here: > >>>> > > >>>> > > >>>> > https://github.com/datibbaw/php-src/compare/php:PHP-5.4...bug66834a > >>>> > > >>>> > >>>> > > >>>> > It's suffering from a few memory leaks which can be fixed, > >>>> > > >>>> > >>>> They are fixed now and a new PR has been opened: > >>>> > >>>> https://github.com/php/php-src/pull/616 > >>>> > >>>> > >>> Hi Etienne, > >>> > >>> Are you fine with the changes in the PR? > >>> > >>> > >> Two things: > >> > >> 1) Where is this planned to be merged? Is it really 5.4 ? As is the PR > is > >> no longer a simple bug fix and while I agree it should be changed that > way, > >> it might introduce hard-to-track BC breaks. > >> > > > > After a short discussion on IRC I'm moving the branch to target 5.6 > > instead. > > > > > >> 2) Seems to me that with this PR, part of the hack present in > ArrayObject > >> in the has_dimension handler is no longer necessary, as it might be > doing > >> to value-check twice now. > >> > > > > I'll start looking into this now. Thanks for the heads up :) > > > > I've updated and cleaned up ext/spl; the most notable change is code > removal according to a changed definition of zend_object_has_dimension_t: > > typedef int (*zend_object_has_dimension_t)(zval *object, zval *member > TSRMLS_DC); > > PR: https://github.com/php/php-src/pull/621 > > Let me know if you see any other issues; I'm fine with reverting the > typedef change if that could be an issue. > > > > > > > >> > >> > >> Best, > >> > >> -- > >>> Ferenc Kovács > >>> @Tyr43l - http://tyrael.hu > >>> > >> > >> > >> > >> -- > >> Etienne Kneuss > >> http://www.colder.ch > >> > > > > > > > > -- > > -- > > Tjerk > > > > > > -- > -- > Tjerk >