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

Commit reverted via https://github.com/php/php-src/commit/171c759d791f809ebc31711fd0b0b5bb632cd2cc . Nikita On Sat, Jul 16, 2016 at 5:21 PM, Matteo Beccati <php@beccati.com> wrote: > 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/ > > -- > PHP CVS Mailing List (http://www.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php >