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

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.

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;
>  }
> 
> 
>