Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Sent from my iPhone 在 2012-4-18，3:22，Yasuo Ohgaki 写道： > Hi, > > 2012/4/17 Johannes Schlüter : >> On Sun, 2012-04-15 at 13:00 +0800, Laruence wrote: >>> On Sun, Apr 15, 2012 at 12:36 PM, Stas Malyshev >>> wrote: Hi! >> I'm not sure this is a good idea - output buffers can include output >> functions IIRC, but you have already shut down modules. I would be very, > Stas, they are request shutdown, not module shutdown :) Request shutdown IIRC destroys functions and module's per-request data structures, which user-supplied output handlers may need. Did you check that user-supplied output handlers - especially those that use extensions and per-request data structures - actually work after this change? >>> >>> Ah, thanks, I got your point. but output handler is called in step 3: >>> /* 3. Flush all output buffers */ >>> >>> and as I said, all tests passed. >>> >>> anyway, you are right, we should be careful, I will double check it. :) >> >> I can't find something that breaks, but we should be really really >> really really careful with this Johannes, I got you point, I checked again, the out buffer deactivate function now only release output handler stack, since mike rewritten the whole implement. So i think this fix seems a appropriate fix:) >> shutdown order. changes there caused >> trouble in the past and I rather have a error message from output >> buffers saying "output buffer can't be used in shutdown" than changing >> the order ... >> > > It would be nice to fix this without error if it works. > How about gave it a chance for next bug fix release? > (i.e. 5.4.2) > yes, except the crash, this fix also fixed a bug that anything output in request shutdown phase will not send to client. Thanks > Regards, > > -- > Yasuo Ohgaki > yohg...@ohgaki.net -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Hi, 2012/4/17 Johannes Schlüter : > On Sun, 2012-04-15 at 13:00 +0800, Laruence wrote: >> On Sun, Apr 15, 2012 at 12:36 PM, Stas Malyshev >> wrote: >> > Hi! >> > >> >>> I'm not sure this is a good idea - output buffers can include output >> >>> functions IIRC, but you have already shut down modules. I would be very, >> >> Stas, they are request shutdown, not module shutdown :) >> > >> > Request shutdown IIRC destroys functions and module's per-request data >> > structures, which user-supplied output handlers may need. Did you check >> > that user-supplied output handlers - especially those that use >> > extensions and per-request data structures - actually work after this >> > change? >> >> Ah, thanks, I got your point. but output handler is called in step 3: >> /* 3. Flush all output buffers */ >> >> and as I said, all tests passed. >> >> anyway, you are right, we should be careful, I will double check it. :) > > I can't find something that breaks, but we should be really really > really really careful with this shutdown order. changes there caused > trouble in the past and I rather have a error message from output > buffers saying "output buffer can't be used in shutdown" than changing > the order ... > It would be nice to fix this without error if it works. How about gave it a chance for next bug fix release? (i.e. 5.4.2) Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
On Sun, 2012-04-15 at 13:00 +0800, Laruence wrote: > On Sun, Apr 15, 2012 at 12:36 PM, Stas Malyshev > wrote: > > Hi! > > > >>> I'm not sure this is a good idea - output buffers can include output > >>> functions IIRC, but you have already shut down modules. I would be very, > >> Stas, they are request shutdown, not module shutdown :) > > > > Request shutdown IIRC destroys functions and module's per-request data > > structures, which user-supplied output handlers may need. Did you check > > that user-supplied output handlers - especially those that use > > extensions and per-request data structures - actually work after this > > change? > > Ah, thanks, I got your point. but output handler is called in step 3: > /* 3. Flush all output buffers */ > > and as I said, all tests passed. > > anyway, you are right, we should be careful, I will double check it. :) I can't find something that breaks, but we should be really really really really careful with this shutdown order. changes there caused trouble in the past and I rather have a error message from output buffers saying "output buffer can't be used in shutdown" than changing the order ... johannes > thanks > > > > -- > > Stanislav Malyshev, Software Architect > > SugarCRM: http://www.sugarcrm.com/ > > (408)454-6900 ext. 227 > > > > -- > Laruence Xinchen Hui > http://www.laruence.com/ > -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Mike: you may want to review this fix, http://news.php.net/php.cvs/68772 , thanks On Sun, Apr 15, 2012 at 1:00 PM, Laruence wrote: > On Sun, Apr 15, 2012 at 12:36 PM, Stas Malyshev > wrote: >> Hi! >> I'm not sure this is a good idea - output buffers can include output functions IIRC, but you have already shut down modules. I would be very, >>> Stas, they are request shutdown, not module shutdown :) >> >> Request shutdown IIRC destroys functions and module's per-request data >> structures, which user-supplied output handlers may need. Did you check >> that user-supplied output handlers - especially those that use >> extensions and per-request data structures - actually work after this >> change? > > Ah, thanks, I got your point. but output handler is called in step 3: > /* 3. Flush all output buffers */ > > and as I said, all tests passed. > > anyway, you are right, we should be careful, I will double check it. :) > > thanks >> >> -- >> Stanislav Malyshev, Software Architect >> SugarCRM: http://www.sugarcrm.com/ >> (408)454-6900 ext. 227 > > > > -- > Laruence Xinchen Hui > http://www.laruence.com/ -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
On Sun, Apr 15, 2012 at 12:36 PM, Stas Malyshev wrote: > Hi! > >>> I'm not sure this is a good idea - output buffers can include output >>> functions IIRC, but you have already shut down modules. I would be very, >> Stas, they are request shutdown, not module shutdown :) > > Request shutdown IIRC destroys functions and module's per-request data > structures, which user-supplied output handlers may need. Did you check > that user-supplied output handlers - especially those that use > extensions and per-request data structures - actually work after this > change? Ah, thanks, I got your point. but output handler is called in step 3: /* 3. Flush all output buffers */ and as I said, all tests passed. anyway, you are right, we should be careful, I will double check it. :) thanks > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Hi! >> I'm not sure this is a good idea - output buffers can include output >> functions IIRC, but you have already shut down modules. I would be very, > Stas, they are request shutdown, not module shutdown :) Request shutdown IIRC destroys functions and module's per-request data structures, which user-supplied output handlers may need. Did you check that user-supplied output handlers - especially those that use extensions and per-request data structures - actually work after this change? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
On Sun, Apr 15, 2012 at 6:07 AM, Stas Malyshev wrote: > Hi! > >> Hi: >> you may want to review this patch. >> >> I have make sure all tests passed, but still not sure is there any >> side-effect. > > I'm not sure this is a good idea - output buffers can include output > functions IIRC, but you have already shut down modules. I would be very, Stas, they are request shutdown, not module shutdown :) thanks > very careful with changing shutdown order. Especially in stable version. > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 -- Laruence Xinchen Hui http://www.laruence.com/ -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Hi! > Hi: >you may want to review this patch. > >I have make sure all tests passed, but still not sure is there any > side-effect. I'm not sure this is a good idea - output buffers can include output functions IIRC, but you have already shut down modules. I would be very, very careful with changing shutdown order. Especially in stable version. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Hi:
you may want to review this patch.
I have make sure all tests passed, but still not sure is there any
side-effect.
thanks
On Sun, Apr 15, 2012 at 1:16 AM, Xinchen Hui wrote:
> Commit: 3b42f184cdcf512fdc1f944052bfa296f17a035f
> Author: Xinchen Hui Sun, 15 Apr 2012 01:16:34
> +0800
> Parents: 6ecac269728360180a2813e75dfbe8338a05a27a
> Branches: PHP-5.4
>
> Link:
> http://git.php.net/?p=php-src.git;a=commitdiff;h=3b42f184cdcf512fdc1f944052bfa296f17a035f
>
> Log:
> Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx)
>
> Bugs:
> https://bugs.php.net/61728
>
> Changed paths:
> M NEWS
> A ext/session/tests/bug61728.phpt
> M main/main.c
>
>
> Diff:
> diff --git a/NEWS b/NEWS
> index 05cc254..2b65382 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -17,6 +17,7 @@ PHP
> NEWS
> (merge after 5.3.11 release)
>
> - Core:
> + . Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx). (Laruence)
> . Fixed bug #61660 (bin2hex(hex2bin($data)) != $data). (Nikita Popov)
> . Fixed bug #61650 (ini parser crashes when using ${} ini variables
> (without apache2)). (Laruence)
> diff --git a/ext/session/tests/bug61728.phpt b/ext/session/tests/bug61728.phpt
> new file mode 100644
> index 000..30b876e
> --- /dev/null
> +++ b/ext/session/tests/bug61728.phpt
> @@ -0,0 +1,39 @@
> +--TEST--
> +Bug #61728 (php-fpm SIGSEGV running friendica on nginx)
> +--SKIPIF--
> +
> +--FILE--
> + +function output_html($ext) {
> + return strlen($ext);
> +}
> +
> +function open ($save_path, $session_name) {
> + return true;
> +}
> +
> +function close() {
> + return true;
> +}
> +
> +function read ($id) {
> +}
> +
> +function write ($id, $sess_data) {
> + ob_start("output_html");
> + echo "laruence";
> + ob_end_flush();
> + return true;
> +}
> +
> +function destroy ($id) {
> +}
> +
> +function gc ($maxlifetime) {
> + return true;
> +}
> +
> +session_set_save_handler ("open", "close", "read", "write", "destroy", "gc");
> +session_start();
> +--EXPECTF--
> +8
> diff --git a/main/main.c b/main/main.c
> index 6a04ddb..c34f952 100644
> --- a/main/main.c
> +++ b/main/main.c
> @@ -1740,22 +1740,22 @@ void php_request_shutdown(void *dummy)
> }
> } zend_end_try();
>
> - /* 4. Shutdown output layer (send the set HTTP headers, cleanup
> output handlers, etc.) */
> - zend_try {
> - php_output_deactivate(TSRMLS_C);
> - } zend_end_try();
> -
> - /* 5. Reset max_execution_time (no longer executing php code after
> response sent) */
> + /* 4. Reset max_execution_time (no longer executing php code after
> response sent) */
> zend_try {
> zend_unset_timeout(TSRMLS_C);
> } zend_end_try();
>
> - /* 6. Call all extensions RSHUTDOWN functions */
> + /* 5. Call all extensions RSHUTDOWN functions */
> if (PG(modules_activated)) {
> zend_deactivate_modules(TSRMLS_C);
> php_free_shutdown_functions(TSRMLS_C);
> }
>
> + /* 6. Shutdown output layer (send the set HTTP headers, cleanup
> output handlers, etc.) */
> + zend_try {
> + php_output_deactivate(TSRMLS_C);
> + } zend_end_try();
> +
> /* 7. Destroy super-globals */
> zend_try {
> int i;
>
>
> --
> 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 CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-CVS] com php-src: Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx): NEWS ext/session/tests/bug61728.phpt main/main.c
Commit:3b42f184cdcf512fdc1f944052bfa296f17a035f
Author:Xinchen Hui Sun, 15 Apr 2012 01:16:34
+0800
Parents: 6ecac269728360180a2813e75dfbe8338a05a27a
Branches: PHP-5.4
Link:
http://git.php.net/?p=php-src.git;a=commitdiff;h=3b42f184cdcf512fdc1f944052bfa296f17a035f
Log:
Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx)
Bugs:
https://bugs.php.net/61728
Changed paths:
M NEWS
A ext/session/tests/bug61728.phpt
M main/main.c
Diff:
diff --git a/NEWS b/NEWS
index 05cc254..2b65382 100644
--- a/NEWS
+++ b/NEWS
@@ -17,6 +17,7 @@ PHP
NEWS
(merge after 5.3.11 release)
- Core:
+ . Fixed bug #61728 (php-fpm SIGSEGV running friendica on nginx). (Laruence)
. Fixed bug #61660 (bin2hex(hex2bin($data)) != $data). (Nikita Popov)
. Fixed bug #61650 (ini parser crashes when using ${} ini variables
(without apache2)). (Laruence)
diff --git a/ext/session/tests/bug61728.phpt b/ext/session/tests/bug61728.phpt
new file mode 100644
index 000..30b876e
--- /dev/null
+++ b/ext/session/tests/bug61728.phpt
@@ -0,0 +1,39 @@
+--TEST--
+Bug #61728 (php-fpm SIGSEGV running friendica on nginx)
+--SKIPIF--
+
+--FILE--
+http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
