Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24049
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Fri, 9 Jun 2006 21:10:55 +0300 (IDT)
To: Steph Fox <steph@zend.com>
cc: internals@lists.php.net
In-Reply-To: <165e01c68be9$afb4cb60$6602a8c0@foxbox>
Message-ID: <Pine.LNX.4.64.0606092059200.17062@shire.zend.office>
References: <000001c68af5$50876930$6e02a8c0@thinkpad><7B45819D-6E8D-4B9B-A02B-BC00BE6B1930@prohost.org>
	<7.0.1.0.2.20060608164415.06b4ae50@zend.com> <133f01c68b73$fb1a1bd0$6602a8c0@foxbox>
	<Pine.LNX.4.64.0606091127001.17284@mail.zend.com> <145901c68bad$cdd4b470$6602a8c0@foxbox>
	<Pine.LNX.4.64.0606091732500.306@mail.zend.com> <165e01c68be9$afb4cb60$6602a8c0@foxbox>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
Subject: Re: [PHP-DEV] Re: [PATCH] Automatic module globals management
From: stas@zend.com (Stanislav Malyshev)

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