Re: Re: [PATCH] Automatic module globals management

SF>>Because we need to work through one resource at a time from the top, SF>>and because dl'd modules are the last things with a resource id to be SF>>loaded, because they have to be. As you see belowe, this is not correct. SF>>You got me there. OK, I didn't know that happens until just now. The rest is SF>>fine - really. But ... do Zend extensions actually use TSRM at all? - I Sure they do. Why shouldn't they? SF>>I already went through that twice today... And not once the answer "well, they are going to die anyway, so who cares if we free ID for wrong module?" was satisfactory. I think it's not an acceptable behaviour for the engine. SF>>You're missing some crucial bits. You're assuming that the id that is freed SF>>is actually that module's id, for a start. It isn't, it's just the next id That's wrong assumption, because it bases on assumption that each module allocates exactly one ID and only modules allocate IDs. Both of these are wrong. SF>>Yes, but since they're loading earlier than modules there's no impact SF>>there. Who said all extensions would load before all modules? What if extension loads a module? SF>>You're misunderstanding me. The only thing that can happen is that the SF>>next module in the list has its resource_id freed near the end of a SF>>loop rather than at the start of the next loop. Because the count only SF>>considers dl'd modules, at worst that means a couple of the builtin SF>>extensions might get freed 'early' in that sense. Half of them already SF>>call ts_free_id() anyway - the impact of doing that is absolutely not SF>>negative. The code I see in the patch as I see it may very well call ts_free_id for a module TSRM resource before module->module_shutdown_func is called, if the formula (table_size - zend_dl_module_count()) gets wrong id. As there never was a condition that shutdown_func may not use globals before they are destroyed, the result would be module attempting to access already free resource. Please explain why this situation is not possible - I see the code is in module_destructor as follows: if (module->module_started && module->module_shutdown_func) { module->module_shutdown_func(module->type, module->module_number TSRMLS_CC); } module->module_started=0; /.../ if (module->handle) { if (!module->globals_id_ptr) { /.../ ts_free_id(table_size - zend_dl_module_count()); } DL_UNLOAD(module->handle); } So there's actually shutdown call between ts_free_id calls. -- Stanislav Malyshev, Zend Products Engineer stas@zend.com http://www.zend.com/ +972-3-6139665 ext.115