Re: com php-src: Fixed bug #61922 (ZTS build doesn't accept zend.script_encoding config): NEWS Zend/zend.c

Hi Dmitry: On Sat, May 5, 2012 at 6:38 PM, Laruence <laruence@php.net> wrote: > On Sat, May 5, 2012 at 2:36 PM, Dmitry Stogov <dmitry@zend.com> wrote: >> Hi Laruence, >> >> Thank you for sending this. >> >> I'm not sure if the patch is completely correct. >> With the patch all the threads share the single copy of script_encoding_list >> and when one thread terminates it calls compiler_globals_dtor() and frees >> the script_encoding_list. But other threads still keep reference to it. >> >> I think we have to duplicate script_encoding_list for each thread in the >> same way as we do for CG(function_table). after a further exam, this is right, there is a mechanism for new thread re-configure inis(zend_ini_refresh_caches). then new thread will have a copy. > right, thanks >> >> Also I noticed a related issue. At zend.c compiler_globals_dtor() >> CG(script_encoding_list) deallocated using free() and in zend_multibyte.c >> zend_multibyte_set_script_encoding() using efree(). >> >> I suppose the second place has to be fixed. >> >> I would appreciate if you could look into the problems. and this should use free, I will fix it . however for now it's dead codes, so no bug feedback. :) thanks > okey, I will, thanks :) > >> >> Thanks. Dmitry. >> >> >> >> On 05/03/2012 06:51 PM, Laruence wrote: >>> >>> Hi, Dmitry: >>> >>>      you may want to review this,  :) >>> >>> thanks >>> On Thu, May 3, 2012 at 10:39 PM, Xinchen Hui<laruence@php.net>  wrote: >>>> >>>> Commit:    72f19e9a8bcf5712b24fa333a26616eff19ac1ce >>>> Author:    Xinchen Hui<laruence@php.net>           Thu, 3 May 2012 >>>> 22:39:53 +0800 >>>> Parents:   d74d88fbb9c29b1dd5ff05a54b72cf7c9250955c >>>> Branches:  PHP-5.4 >>>> >>>> Link: >>>> >>>> http://git.php.net/?p=php-src.git;a=commitdiff;h=72f19e9a8bcf5712b24fa333a26616eff19ac1ce >>>> >>>> Log: >>>> Fixed bug #61922 (ZTS build doesn't accept zend.script_encoding config) >>>> >>>> Bugs: >>>> https://bugs.php.net/61922 >>>> >>>> Changed paths: >>>>  M  NEWS >>>>  M  Zend/zend.c >>>> >>>> >>>> Diff: >>>> diff --git a/NEWS b/NEWS >>>> index 8796cf4..9ef6abf 100644 >>>> --- a/NEWS >>>> +++ b/NEWS >>>> @@ -10,6 +10,8 @@ PHP >>>>                    NEWS >>>>     (Laruence) >>>> >>>>  - Core: >>>> +  . Fixed bug #61922 (ZTS build doesn't accept zend.script_encoding >>>> config). >>>> +    (Laruence) >>>>   . Fixed missing bound check in iptcparse(). (chris at chiappa.net) >>>>   . Fixed bug #61827 (incorrect \e processing on Windows) (Anatoliy) >>>>   . Fixed bug #61761 ('Overriding' a private static method with a >>>> different >>>> diff --git a/Zend/zend.c b/Zend/zend.c >>>> index dd299f1..37a1a27 100644 >>>> --- a/Zend/zend.c >>>> +++ b/Zend/zend.c >>>> @@ -781,6 +781,8 @@ void zend_register_standard_ini_entries(TSRMLS_D) /* >>>> {{{ */ >>>>  void zend_post_startup(TSRMLS_D) /* {{{ */ >>>>  { >>>>  #ifdef ZTS >>>> +       zend_encoding **script_encoding_list; >>>> + >>>>        zend_compiler_globals *compiler_globals = >>>> ts_resource(compiler_globals_id); >>>>        zend_executor_globals *executor_globals = >>>> ts_resource(executor_globals_id); >>>> >>>> @@ -795,7 +797,12 @@ void zend_post_startup(TSRMLS_D) /* {{{ */ >>>>        zend_destroy_rsrc_list(&EG(persistent_list) TSRMLS_CC); >>>>        free(compiler_globals->function_table); >>>>        free(compiler_globals->class_table); >>>> -       compiler_globals_ctor(compiler_globals, tsrm_ls); >>>> +       if ((script_encoding_list = (zend_encoding >>>> **)compiler_globals->script_encoding_list)) { >>>> +               compiler_globals_ctor(compiler_globals, tsrm_ls); >>>> +               compiler_globals->script_encoding_list = (const >>>> zend_encoding **)script_encoding_list; >>>> +       } else { >>>> +               compiler_globals_ctor(compiler_globals, tsrm_ls); >>>> +       } >>>>        free(EG(zend_constants)); >>>>        executor_globals_ctor(executor_globals, tsrm_ls); >>>>        global_persistent_list =&EG(persistent_list); >>>> >>>> >>>> -- >>>> PHP CVS Mailing List (http://www.php.net/) >>>> To unsubscribe, visit: http://www.php.net/unsub.php >>>> >>> >>> >>> >> > > > > -- > Laruence  Xinchen Hui > http://www.laruence.com/ -- Laruence  Xinchen Hui http://www.laruence.com/