php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #39344 Unnecessary calls to OnModify callback routine for an extension INI directive
Submitted: 2006-11-02 11:10 UTC Modified: 2006-11-08 11:05 UTC
From: wharmby at uk dot ibm dot com Assigned: dmitry
Status: Closed Package: Scripting Engine problem
PHP Version: 5CVS-2006-11-02 (snap) OS: Windows XP
Private report: No CVE-ID:
 [2006-11-02 11:10 UTC] wharmby at uk dot ibm dot com
Description:
------------
Unnecessary calls to OnModify callback routine for an
extension INI directives in ZTS enabled builds.

Please note the problem reported here only applies to ZTS 
enabled builds.

I have tried the supplied testcase on the latest snap-shot 
build for Windows (Nov 2, 2006 09:30 GMT)and the problem 
persists. phpinfo() shows "PHP Version => 5.2.1-dev".

Problem also persists in latest checked in version of file 
in CVS.

If I define a OnMOdify callback routine for an extension INI
directive the routine is called multiple times during
startup even though the directive is not being continually
modified. I expected one call as a result of modification to
the directive in php.ini but instead I got 3 calls.

I spotted this behaviour reviewing the engine code whilst 
reading Sara Golemon's book "Extending and Embedding PHP" 
and include a slightly modified version of sample code
from her book to illustrate the unnecessary calls.

The first problem is that in a ZTS enabled build 
zend_ini_refresh_caches() is called twice for each new 
thread. The method zend_new_thread_end_handler() calls it 
directly as follows: 

static void zend_new_thread_end_handler(THREAD_T thread_id TSRMLS_DC)
{
    zend_copy_ini_directives(TSRMLS_C);
    zend_ini_refresh_caches(ZEND_INI_STAGE_STARTUP TSRMLS_CC);
}

However, zend_copy_ini_directives() itself also calls zend_ini_refresh_caches() so we end up calling any OnModifty callback routines twice for each new thread.

I believe that: 
   (1) the call in zend_copy_ini_directives() can safely be
       removed, and 
   (2) the call in zend_new_thread_end_handler() should
       really be conditional on the success of the copy 
       operation, otherwise we could attempt to iterate
       over a non-existent hash and die. 

This will reduce the number of calls to 2 on ZTS emabled builds but any OnModifycallback routine will still be called twice on the startup thread in ZTS enabled builds; once by zend_register_ini_entries() during MINIT processing when an extension registers its INI directives and again on ZTS
builds during zend_post_startup() processing, i.e 

 zend_post_startup() -> zend_new_thread_end_handler()  -> 
 zend_ini_refresh_caches()

Whilst the call to the OnModify callback routine from zend_register_ini_entries() is required for non-ZTS builds 
to work correctly I can see no need for a call from 
zend_ini_refresh_caches() during zend_post-startup 
processing. I believe this can easily be resolved by 
modifying zend_post_startup() to call 
      zend_copy_ini_directives() 
instead of 
      zend_new_thread_end_handler()

My patch for zend.c is here: http://pastebin.ca/234196
and for zend_ini.c here: http://pastebin.ca/234200 


Reproduce code:
---------------
Reproduce code is here: http://pastebin.ca/234201

I tested by adding the following to php.ini:

sample4.greeting="Hello Andy"

or via command line as:

-dsample4.greeting="Hello Andy"

Expected result:
----------------
When running using CLI on Windows, i.e a ZTS enabled build, I 
expected to see 1 call to my extensions "OnModify" callback 
routine for each thread attached; so in this simple case I expected to see just the 1 call as follows:

>> sample 4: thread 0xfbc minit
sample 4: thread 0xfbc greeting modified..new value is Hello Andy
<< sample 4: thread 0xfbc minit
Hello Andy


Actual result:
--------------
The OnModify call back routine is in fact called 3 times; once
during MINIT processing and twice further. These last 2 are during the call to zend_new_thread-end_handler().

>> sample 4: thread 0x16f8 minit
sample 4: thread 0x16f8 greeting modified..new value is Hello Andy
<< sample 4: thread 0x16f8 minit
sample 4: thread 0x16f8 greeting modified..new value is Hello Andy
sample 4: thread 0x16f8 greeting modified..new value is Hello Andy
Hello Andy




Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2006-11-03 09:03 UTC] wharmby at uk dot ibm dot com
Correcting version; behaviour was observed in V5 not V4
 [2006-11-08 11:05 UTC] dmitry@php.net
The patches applied to CVS HEAD and PHP_5_2.
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Thu Apr 17 06:02:13 2014 UTC