why to call php_output_activate(TSRMLS_C) from php_module_startup

php.internals

Kamesh Jayachandran

21 years ago
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? 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? With regards Kamesh Jayachandran

Kamesh Jayachandran

21 years ago
Can someone respoond to this. With regards Kamesh Jayachandran On Thu, 07 Oct 2004 23:10:24 -0700, "Kamesh Jayachandran" <kameshj@fastmail.fm> said:

Zeev Suraski

21 years ago
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

Marcus Börger

21 years ago
Hello Zeev, Monday, October 18, 2004, 11:14:20 AM, you wrote:
> 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.
We also know of a few bug reports that might have their origin here. In the past i tried to fix all situations i could detect and replicate however there might be others since the code has changed a lot. Best regards, Marcus mailto:helly@php.net