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.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | drupal-set-header-318310-3-D7.patch | 1.27 KB | dave reid |
| #1 | drupal-set-header-D7.patch | 598 bytes | dave reid |
Comments
Comment #1
dave reidDrupal lost my patch again. ARG.
Comment #2
gpk commentedMarked #318306: _drupal_full_bootstrap() (or drupal_set_header()) should learn to be more clever as duplicate.
Comment #3
dave reidAlso fixed a PHP error with the static variable in _drupal_full_bootstrap, as well as a small spacing coding standard issue.
Comment #4
gpk commentedWould 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.
Comment #5
Anonymous (not verified) commentedThe patch file contains patches that aren't fixing this issue.
If headers_sent() we should have a watchdog entry.
Comment #6
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().
Comment #7
dave reidgpk, we can send a watchdog entry as late as we want, it's no problem.
Comment #8
gpk commentedAhhhh 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..??
Comment #9
dave reidAs 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.
Comment #10
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.
Comment #11
gpk commentedActually 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() ..
Comment #12
casey commentedCode regarding this issue has changed a lot. Still an issue?