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

So thank you for doing this. It was my fault.

I already thought that I had done similar changes in all necessary
places in the past. :)

Thanks. Dmitry.

Arnaud Le Blanc wrote:
> 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;
>>>  }
>>>
>>>
>>>
>