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

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 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„­ÃKGCf…Ü > Ïæ¿ > > Log: > Fixed bug #72286 (Segmentation fault During Garbage Collection) > > Bugs: > 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/