Re: Re: PHP 7.3 zif_handler changes

On Tue, 10 Jul 2018, Christoph M. Becker wrote: > 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. I've see that patch, but it needs a fair bit more work that I've started on. Once it all compiles, and works, I'll merge it including your commit. cheers, Derick -- https://derickrethans.nl | https://xdebug.org | https://dram.io Like Xdebug? Consider a donation: https://xdebug.org/donate.php, or become my Patron: https://www.patreon.com/derickr twitter: @derickr and @xdebug