Re: Re: ZTS issues

From: Michael Sisolak > It looks like most of these changes are PHP5 specific (either the code > changes like many of the free()s were already in PHP4, or involve > things like the global constants cache which don't exist there) which I > haven't looked at enough to comment on. The only question I have is > about the freeing of the BG(url_adapt_state_ex) tags hash. I see you > added the code to basic_globals_dtor to free it after each request > (which makes sense as it's wiped in basic_globals_ctor), but why only > for the ZTS build? Is there a reason that this couldn't be used for > all the builds (and remove the similar code for non-ZTS-only builds > from the url_scanner shutdown)? Hummm... I also just saw the > php_url_scanner_ex_deactivate() function. Maybe the hash free should > be there instead? Looking at how its laid out, the php_url_scanner_ex_deactivate() is called on request shutdown. The basic_globals_dtor is only called when the thread is cleaned up (either an explicit call to ts_free_thread or tsrm_shutdown). In a ZTS build, the freeing of the BG(url_adapt_state_ex) tags hash doesnt need to happen until the thread is being killed as PHP_INI_MH(OnUpdateTags), called at the start of a request, in url_scanner_ex.c will destroy the hashtable if it has already been malloc'd otherwise it will malloc it. If the hash free were moved to php_url_scanner_ex_deactivate(), then it would always need to be malloc'd on each new request. Though this is an option, it seems the point of ZTS was to reduce the amount of malloc/frees needed and reuse memory (pretty much why I wasnt supposed to add the ts_free_thread calls at the end of the requests). For a non-ZTS build, the hash is destroyed in the PHP_MSHUTDOWN_FUNCTION(url_scanner) and as it should be running on the same thread for the requests, this should be fine. It possibly could be moved to the basic_globals_dtor function as that is called by non-ZTS builds as well in PHP_MSHUTDOWN_FUNCTION(basic). Then it would all be in a common place. Rob