Found a problem after some discussions in #318102: hook_exit() not invoked for some cached requests and #231724: Incompatible with page caching (incomplete cron runs, possible PHP fatal errors). The problem is if drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL) is called within hook_exit() during a cached request, Drupal tries to send another header (_drupal_bootstrap_full) after headers and content have been sent. I've created a patch that if checks headers_sent() is FALSE in drupal_set_header before sending a new header.

CommentFileSizeAuthor
#3 drupal-set-header-318310-3-D7.patch1.27 KBDave Reid
#1 drupal-set-header-D7.patch598 bytesDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
drupal-set-header-D7.patch598 bytes

Drupal lost my patch again. ARG.

gpk’s picture

gpk CreditAttribution: gpk commented
Title: Fatal PHP error with drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL) on cached request » Fatal PHP error with _drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL) on cached request

Marked #318306: _drupal_full_bootstrap() (or drupal_set_header()) should learn to be more clever as duplicate.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
FileSize
drupal-set-header-318310-3-D7.patch1.27 KB

Also fixed a PHP error with the static variable in _drupal_full_bootstrap, as well as a small spacing coding standard issue.

gpk’s picture

gpk CreditAttribution: gpk commented

Would this essentially prevent "Warning: Cannot modify header information - headers already sent by (output started at ...)" messages caused by e.g. premature start of output? If so, could make debugging difficult.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work

The patch file contains patches that aren't fixing this issue.

If headers_sent() we should have a watchdog entry.

gpk’s picture

gpk CreditAttribution: gpk commented

>we should have a watchdog entry
Except that in the case of hook_exit() calling _drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL) leading to this condition then we wouldn't want a watchdog entry.

Maybe the answer is just do do the headers_sent() check in _drupal_bootstrap_full().

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

gpk, we can send a watchdog entry as late as we want, it's no problem.

gpk’s picture

gpk CreditAttribution: gpk commented

Ahhhh I was taking the purpose of this issue to be to allow invocation of a bootstrap (possibly full) from hook_exit() without causing an error ... if a module intentionally does this then I thought we wouldn't want it to be logged in watchdog every time?

If we are in a "fatal PHP error headers already sent output started at ..." situation then generally this indicates a problem with a module somewhere (as in, you gotta bug) and we should IMO let PHP fall over..??

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

As I see it, if this a module is needing to bootstrap to BOOTSTRAP_FULL and headers have already been sent, there should be a better way for the module to go around loading what it needs to, so we should make sure that a watchdog message is provided and the module maintainers can address the issue.

gpk’s picture

gpk CreditAttribution: gpk commented

@9: I'm not quite clear where we are heading with this ... the original post implies that invoking a full bootstrap in hook_exit() should be an OK thing to do, but now the suggestion seems to be that doing this should cause a watchdog entry, i.e. it is not an OK thing to do. Or am I missing the point?!

>there should be a better way for the module to go around loading what it needs to
Indeed, but I'm not sure what it is. You saw how I attempted to do it in poormanscron.module 5.x: http://drupal.org/files/issues/poormanscron-bootstrap-231724-5.x_0.patch .. not exactly very beautiful!

Also I'm not clear that checking headers_sent() is sufficient because for a 304 header response the headers I think won't have been sent yet by the time hook_exit() gets invoked, but we don't really want to be adding the additional header from http://api.drupal.org/api/function/_drupal_bootstrap_full/7 in this case.

gpk’s picture

gpk CreditAttribution: gpk commented

Actually there is a second problem with trying to invoke a full bootstrap from hook_exit() after a cached response, namely that _drupal_bootstrap_full() invokes hook_init() ..

casey’s picture

casey CreditAttribution: casey commented

Code regarding this issue has changed a lot. Still an issue?