Re: com php-src: Fixed bug #72286 (Segmentation fault During Garbage Collection): NEWS Zend/tests/bug72286.phpt Zend/zend_gc.c

Hi Dmitry, I've just tried, but I do not have karma for the Zend directory, so my push has been rejected. I'm afraid someone else has to do it. BTW, are you suggesting to revert it up to master or just in the 5.6 branch? Cheers On 16/07/2016 14:29, Dmitry Stogov wrote: > Hi Matteo, > > Please revert the patch. I can't do it myself now. > Xinchen, could you please try to look into the problem. > > Thanks. Dmitry. > > On Jul 15, 2016 4:28 PM, Matteo Beccati <php@beccati.com> wrote: > > > > >> Hi Dmitry, >> >> Pretty much all the application test suites I'm running were failing >> this morning due to segmentation faults on PHP-5.6 only, e.g.: >> >> >> https://revive.beccati.com/bamboo/browse/PHP-PHPSPEC-PHP56-772 > > <https://revive.beccati.com/bamboo/browse/PHP-PHPSPEC-PHP56-772> >> >> If I revert this commit, the tests get green again. The easiest way to >> replicate is to get phpspec master and running its own test suite: >> "bin/phpspec run". >> >> >> Cheers >> >> On 14/07/2016 21:37, Dmitry Stogov wrote: >> > Commit: 1c84b55adea936b065a20102202bea3d1d243225 >> > Author: Dmitry Stogov <dmitry@zend.com> Thu, 14 Jul 2016 > 22:37:25 +0300 >> > Parents: d0c98366d08d96e3b364e744fca76cc98a0d7f15 >> > Branches: PHP-5.6 PHP-7.0 master >> > >> > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=1c84b55adea936b065a20102202bea3d1d243225 > > <http://git.php.net/?p=php-src.git;a=commitdiff;h=1c84b55adea936b065a20102202bea3d1d243225> >> > >> > Log: >> > Fixed bug #72286 (Segmentation fault During Garbage Collection) >> > >> > Bugs: >> >https://bugs.php.net/72286 >> ><https://bugs.php.net/72286> >> > >> > Changed paths: >> > M NEWS >> > A Zend/tests/bug72286.phpt >> > M Zend/zend_gc.c >> > >> > >> > Diff: >> > diff --git a/NEWS b/NEWS >> > index a9d242b..a9ca67a 100644 >> > --- a/NEWS >> > +++ b/NEWS >> > @@ -5,6 +5,7 @@ > PHP > NEWS >> > - Core: >> > . Fixed bug #72581 (previous property undefined in Exception after >> > deserialization). (Laruence) >> > + . Fixed bug #72286 (Segmentation fault During Garbage > Collection). (Dmitry) >> > . Fixed bug #72024 (microtime() leaks memory). (maroszek at gmx > dot net) >> > >> > - Curl: >> > diff --git a/Zend/tests/bug72286.phpt b/Zend/tests/bug72286.phpt >> > new file mode 100644 >> > index 0000000..0f6184d >> > --- /dev/null >> > +++ b/Zend/tests/bug72286.phpt >> > @@ -0,0 +1,51 @@ >> > +--TEST-- >> > +Bug #72286 (Segmentation fault During Garbage Collection) >> > +--FILE-- >> > +<?php >> > +class SegfaultScenario >> > +{ >> > + private $circular_reference; >> > + private $object; >> > + >> > + public function __construct() >> > + { >> > + $this->circular_reference = $this; >> > + $this->object = new \stdClass(); >> > + } >> > + >> > + public function __destruct() >> > + { >> > + // we try to avoid accessing $this->object by returning > early but the object exists >> > + if (!$this->object) { // without this expression involving > the object, the segfault does not happen >> > + var_dump('the object exists'); >> > + return; >> > + } >> > + >> > + var_dump('segfaults here:'); >> > + // and then access the object (which seemingly has already > been garbage collected) >> > + var_dump($this->object); >> > + var_dump('will not get here'); >> > + } >> > +} >> > + >> > +class SomeContainer >> > +{ >> > + public function run() >> > + { >> > + new SegfaultScenario(); >> > + } >> > +} >> > + >> > +$container = new SomeContainer(); >> > +$container->run(); >> > + >> > +var_dump('we are about to segfault'); >> > +gc_collect_cycles(); >> > +var_dump('will not get here'); >> > +--EXPECTF-- >> > +string(24) "we are about to segfault" >> > +string(15) "segfaults here:" >> > +object(stdClass)#%d (0) { >> > +} >> > +string(17) "will not get here" >> > +string(17) "will not get here" >> > diff --git a/Zend/zend_gc.c b/Zend/zend_gc.c >> > index e72655c..7ad7340 100644 >> > --- a/Zend/zend_gc.c >> > +++ b/Zend/zend_gc.c >> > @@ -650,6 +650,7 @@ tail_call: >> > >> > if (!props) { >> > /* restore refcount > and put into list to free */ >> > + obj->refcount = 1; >> > pz->refcount__gc++; >> > > ((zval_gc_info*)pz)->u.next = GC_G(zval_to_free); >> > GC_G(zval_to_free) = > (zval_gc_info*)pz; >> > @@ -756,6 +757,7 @@ static void gc_collect_roots(TSRMLS_D) >> > struct _store_object *obj = > &EG(objects_store).object_buckets[current->handle].bucket.obj; >> > zval z; >> > >> > + obj->refcount = 1; >> > GC_SET_ADDRESS(obj->buffered, NULL); >> > INIT_PZVAL(&z); >> > Z_OBJ_HANDLE(z) = current->handle; >> > @@ -802,7 +804,7 @@ ZEND_API int gc_collect_cycles(TSRMLS_D) >> > if (Z_TYPE(p->z) == IS_OBJECT) { >> > if (EG(objects_store).object_buckets && >> > > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].valid && >> > - > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount > <= 0 && >> > + > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount > <= 1 && >> > > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.dtor && >> > > !EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].destructor_called) { >> > >> > @@ -823,7 +825,7 @@ ZEND_API int gc_collect_cycles(TSRMLS_D) >> > if (Z_TYPE(p->z) == IS_OBJECT) { >> > if (EG(objects_store).object_buckets && >> > > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].valid && >> > - > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount > <= 0) { >> > + > --EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount > <= 0) { >> > > EG(objects_store).object_buckets[Z_OBJ_HANDLE(p->z)].bucket.obj.refcount > = 1; >> > Z_TYPE(p->z) = IS_NULL; >> > > zend_objects_store_del_ref_by_handle_ex(Z_OBJ_HANDLE(p->z), > Z_OBJ_HT(p->z) TSRMLS_CC); >> > >> > >> >> >> -- >> Matteo Beccati >> >> Development & Consulting -http://www.beccati.com/ > <http://www.beccati.com/> > -- Matteo Beccati Development & Consulting - http://www.beccati.com/