Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:26741
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain googlemail.com designates 66.249.92.170 as permitted sender)
DomainKey-Status: good
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=googlemail.com;
        h=received:message-id:date:from:user-agent:mime-version:to:subject:references:in-reply-to:content-type:content-transfer-encoding;
        b=pk+uscyKl3pJAwSHX24OxUKbnvuJLVNyYbQ+OrasO/6xu9kkPazItr/W37UmT1v5y6UM2wTSEud1EHz7+K6YWObzIfLZkKctjZ/f+vuw4O6q5ET+cNpaOCaHZgCE4vGksIYLs4Czite7jLMMkvYMMSupfCmZFH/cyJxXN8VsWfk=
Message-ID: <456EEF62.5040209@googlemail.com>
Date: Thu, 30 Nov 2006 14:49:06 +0000
User-Agent: Thunderbird 1.5.0.8 (Windows/20061025)
MIME-Version: 1.0
To: PHP internals list <internals@lists.php.net>
References: <456D8D30.7070405@googlemail.com> <456E06A2.60007@zend.com>
In-Reply-To: <456E06A2.60007@zend.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Question on thread safety
From: andy.wharmby@googlemail.com (Andy Wharmby)

Stanislav Malyshev wrote:
>> So finally to my question.  Is it the intention of TSRMc. to allow 
>> ts_allocate_id() to be called at any time or is there an unwritten 
>> rule that it
>> should only ever called during php startup ? If its the former then I 
>
> I think it gets called only on startup. I also think it was the 
> intent, though there is no safeguard as far as I can see against 
> calling it in run-time, but no module does it and it doesn't make 
> sense to do it in other place than startup.
>
>> I myself see no reason why extension writers should be restricted 
>> from calling ts_allocate_id() outside PHP startup so believe the code 
>> needs 
>
> Well, the reason is that if you want to use TSRM globals, you have to 
> allocate ID before you do basically anything with them. Startup is a 
> good place for that. If you don't need globals, then you should not 
> call it at all. The situation where in the mid-run you suddenly 
> remember you need globals seems quite unrealistic to me. Of course, if 
> you can describe scenario when you would really need it in mid-run or 
> it would make sense to allocate ID in mid-run, then I guess this 
> should be fixed or at least safeguarded.
I did not have any particular scenario in mind here as I too could not 
come up with a sensible reason to allow calls to ts_allocate_id() 
outside initialization. Initially I thought of the case of a user script
loading an extension using dl() but soon found out this is policed  and 
not allowed if ZTS is enabled. The reason I assumed that 
ts_allocate_id() was designed to be called at any time was the fact that 
the code is wrapped in a mutex, which lead to my concerns about 
ts_resource_ex(). If ts_allocate_id() is not designed to be called 
outside startup then my concerns about ts_resource_ex() are unfounded. 
However, the mutex acquire and release calls in ts_allocate_id()  are 
therefore unnecessary and should be removed.

However, I do believe this restriction should be policed to fail any 
calls outside startup. I see nothing in the code to stop a extension 
writer calling ts_allocate_id at runtime. Why would anyone do this ? So 
TSRM global  storage is only allocated when an extension is actually 
needed rather than when a new thread is started. What if a user has an 
extension that only gets called in some exceptional circumstance and 
they design it so the ts_allocate_id() call is made on the first call to 
the extension to save storage being allocated for threads until its 
needed. Unlikely I know but  sometimes users do what you least
expect so the code should protect them wherever possible from them doing 
something which will:
    (a) cause them grief, and
    (b) probably lead them into raising a bogus defect and waste 
someones time diagnosing the problem
A simple check in the code could prevent all this.

There are further routines in TSRM.c which also acquire the tsmm_mutex 
were the reason for this is not clear given current usage:

    * ts_free_id(). Only called at MSHUTDOWN when single threaded so no
      apparent need for mutex.  Again easily policed to ensure calls
      outside startup/shutdown not allowed.
    * ts_free_worker_threads():  Only called by php_module_shutdown when
      single threaded so no need for mutex.
    * ts_resource_ex: Needs mutex whilst it updates tsrm_tls_table for a
      new thread but it looks like it keeps mutex longer than it need
      do. By reworking this routine and allocate_new_resource() I
      believe the time the mutex is held could be reduced.

I am happy to work on a patch for all this and will raise a defect with 
patch when I have something worthy of consideration..