Re: Re: PHP 7.3 zif_handler changes

On 09.07.2018 at 11:41, Derick Rethans wrote: > On Wed, 4 Jul 2018, Christoph M. Becker wrote: > >> On 04.07.2018 at 12:24, Nikita Popov wrote: >> >>> On Wed, Jul 4, 2018 at 11:32 AM, Christoph M. Becker <cmbecker69@gmx.de> >>> wrote: >>> >>>> On 04.07.2018 at 01:16, Jan Ehrhardt wrote: >>>> >>>>> Another one: some extensioms suffer from the fact that the macro GC_G >>>>> was removed from >>>>> >>>>> https://github.com/php/php-src/blob/master/Zend/zend_gc.h >>>>> Some of the removed functionality has been reintroduced in gc_status >>>>> >>>>> https://github.com/php/php-src/commits/master/Zend/zend_gc.h >>>>> >>>>> Examples using GC_G: v8js, xdebug, tideways. >>>>> v8js was using gc_active to check if garbage collection was running. >>>>> I did a wild guess to fix it: >>>>> >>>>> https://github.com/Jan-E/v8js/commit/997df065d3cd06a9b11e399458c391 >>>> eb797a850e#diff-dc446a69201ccda44a33d111152f6c8c >>>> >>>> This looks wrong, since zend_gc_collect_cycles() actually triggers the >>>> GC, if I'm not mistaken. >>>> >>>> It seems to me that should be something like: >>>> >>>> zend_gc_status status; >>>> zend_gc_get_status(&status); >>>> if (status.runs) { >>>> >>> >>> The correct way to handle this is to implement a proper get_gc() handler, >>> which will be called in favor of get_properties() during GC. >> >> Ah, thanks! In some other cases, for instance, in Xdebug[1], using >> zend_gc_get_status() would be correct, wouldn't it? >> >> [1] >> >> <https://github.com/xdebug/xdebug/blob/27e20b16d23ab611d2a8e74d2e3c061710e18d8f/xdebug_gc_stats.c#L47-L57> > > It should be - care to make a patch? :-) Xdebug also doesn't do the hash > protections correctly yet. I've not had the time to make a fix for that > yet either (hint!) ;-) I'm already working on that. I'll submit a (partial?) PR ASAP. -- Christoph M. Becker