Re: why to call php_output_activate(TSRMLS_C) from php_module_startup(reposting first time)(bug #30397)

At 08:52 18/10/2004, Kamesh Jayachandran wrote:

Can someone respoond to this. With regards Kamesh Jayachandran On Thu, 07 Oct 2004 23:10:24 -0700, "Kamesh Jayachandran" <kameshj@fastmail.fm> said: Hi All, I could see php_output_activate(TSRMLS_C) getting called from php_module_startup immedeately after php_output_startup. Why is this needed? As each SAPI modules call php_output_activate(TSRMLS_C) explicitly as a part of request startup. Why am I concerned about this? Ans: php_output_activate(TSRMLS_C) sets OG(php_body_write) = php_ub_body_write; which causes calls to php_printf to use php_ub_body_write which depends on SG(request_info) which is null while php_printf is invoked as part of apache startup error(Like Invalid extension/ Non existent extension) logging. This causes segmentation fault. This change seems to have been introduced in version 1.371 of main/main.c by Zeev(3 years and 3 months ago). The comment says, Fix a major thread safety bug in the output mechanism @- Fixed a major memory corruption bug in the thread safe version (Zeev) Can I remove a call to php_output_activate(TSRMLS_C) from php_module_startup?

Actually this existed beforehand, the change just shifted code around (the point of this commit was actually NOT to call php_output_startup() on every request startup, and it introduced php_output_activate() which is the one being called on every request startup instead). From looking at the source code it appears that the call to php_output_activate() during php_module_startup() was added to keep 'compatibility' with the code prior to this commit. It looks that this commit fixed the main problem but kept a minor one. I *think* it can be safely removed.

PS:What is the use of display_startup_errors? What does display mean with respect to startup as it might not have any associated request context?

It has to do with errors that happen during request startup (such as file upload related errors) which cannot be trapped in userland. The check currently present php_error_cb() appears to be messy, though - it should probably be fixed to be just: if (PG(display_errors)

        && module_initialized
        && (!PG(during_request_startup) || PG(display_startup_errors)) {

This messiness may be related to the other issue you mentioned. Zeev