Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2 / zend_gc.c

Hi,

On Monday 15 September 2008 12:18:58 Dmitry Stogov wrote:
> Hi Arnaud,
> 
> I afraid you patch might change the behavior of some scripts.
> At least after the following code you'll probably have invalid $GLOBALS
> refcount.
> 
> <?php
> $a = array();
> $a[0] =& $a;
> $a[1] =& $GLOBALS; // refcount++
> unset($a);
> var_dump(gc_collect_cycles()); // you have disabled refcount--
> ?>
> 
> Were you going to fix a bug?
> Could you please show a test case?
> 
> I might be wrong as I did just source review and didn't run any tests
> yet. I just very worried about any GC changes as they can be reasons of
> errors which are very hard to catch.

This is a merge from PHP_5_3 of the following commit:
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_gc.c#rev1.1.2.10

It fixes the following test: Zend/tests/gc_010.phpt (valgrind results: 
http://gcov.php.net/viewer.php?version=PHP_HEAD&func=valgrind&file=Zend%2Ftests%2Fgc_010.phpt 
)

> 
> Thanks. Dmitry.
> 
> Arnaud Le Blanc wrote:
> > lbarnaud		Sun Sep 14 15:20:16 2008 UTC
> > 
> >   Modified files:              
> >     /ZendEngine2	zend_gc.c 
> >   Log:
> >   MFB "Disabled $GLOBALS' refcount modification"
> >   
> >   
> > 
http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_gc.c?r1=1.17&r2=1.18&diff_format=u
> > Index: ZendEngine2/zend_gc.c
> > diff -u ZendEngine2/zend_gc.c:1.17 ZendEngine2/zend_gc.c:1.18
> > --- ZendEngine2/zend_gc.c:1.17	Sun Aug 24 16:45:50 2008
> > +++ ZendEngine2/zend_gc.c	Sun Sep 14 15:20:16 2008
> > @@ -17,7 +17,7 @@
> >     
+----------------------------------------------------------------------+
> >  */
> >  
> > -/* $Id: zend_gc.c,v 1.17 2008/08/24 16:45:50 helly Exp $ */
> > +/* $Id: zend_gc.c,v 1.18 2008/09/14 15:20:16 lbarnaud Exp $ */
> >  
> >  #include "zend.h"
> >  #include "zend_API.h"
> > @@ -302,7 +302,9 @@
> >  
> >  static int children_scan_black(zval **pz TSRMLS_DC)
> >  {
> > -	(*pz)->refcount__gc++;
> > +	if (Z_TYPE_PP(pz) != IS_ARRAY || Z_ARRVAL_PP(pz) != &EG(symbol_table)) {
> > +		(*pz)->refcount__gc++;
> > +	}
> >  
> >  	if (GC_ZVAL_GET_COLOR(*pz) != GC_BLACK) {
> >  		zval_scan_black(*pz TSRMLS_CC);
> > @@ -346,7 +348,9 @@
> >  
> >  static int children_mark_grey(zval **pz TSRMLS_DC)
> >  {
> > -	(*pz)->refcount__gc--;
> > +	if (Z_TYPE_PP(pz) != IS_ARRAY || Z_ARRVAL_PP(pz) != &EG(symbol_table)) {
> > +		(*pz)->refcount__gc--;
> > +	}
> >  	zval_mark_grey(*pz TSRMLS_CC);
> >  	return 0;
> >  }
> > 
> > 
> > 
>