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

Dmitry Stogov — Thu, 10 May 2012 11:30:01 +0200

On 05/05/2012 06:57 PM, Laruence wrote:
> Hi Dmitry:
>
> On Sat, May 5, 2012 at 6:38 PM, Laruence wrote:
>> On Sat, May 5, 2012 at 2:36 PM, Dmitry Stogov 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.

ok. In case you tested it, then your patch is fine.

Thanks. Dmitry.

>> 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 wrote:
>>>>>
>>>>> Commit: 72f19e9a8bcf5712b24fa333a26616eff19ac1ce
>>>>> Author: Xinchen Hui 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/
>
>
>

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

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

Laruence — Sat, 05 May 2012 17:00:02 +0200

Hi Dmitry:

On Sat, May 5, 2012 at 6:38 PM, Laruence wrote:
> On Sat, May 5, 2012 at 2:36 PM, Dmitry Stogov 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 wrote:
>>>>
>>>> Commit: 72f19e9a8bcf5712b24fa333a26616eff19ac1ce
>>>> Author: Xinchen Hui 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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

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

Laruence — Sat, 05 May 2012 12:40:02 +0200

On Sat, May 5, 2012 at 2:36 PM, Dmitry Stogov 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).
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.
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 wrote:
>>>
>>> Commit: 72f19e9a8bcf5712b24fa333a26616eff19ac1ce
>>> Author: Xinchen Hui 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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

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

Dmitry Stogov — Sat, 05 May 2012 08:40:02 +0200

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).

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.

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 wrote:
>> Commit: 72f19e9a8bcf5712b24fa333a26616eff19ac1ce
>> Author: Xinchen Hui 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
>>
>
>
>

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php