Because the $phase_index++ is after the _drupal_bootstrap call, if a hook_boot module calls back drupal_bootstrap (to boot forward to _PATH) it will break. Also the code can be simplified a tad bit.

CommentFileSizeAuthor
#3 bootstrap-reentrant.patch980 bytesbjaspan
bootstrap_29.patch994 byteschx

Comments

dries’s picture

Status: Needs review » Fixed

This looks OK. Committed.

Anonymous’s picture

Status: Fixed » Closed (fixed)
bjaspan’s picture

Version: 6.x-dev » 5.x-dev
Assigned: chx » bjaspan
Status: Closed (fixed) » Needs review
StatusFileSize
new980 bytes

This is a pretty serious bug and it exists in D5, too. I've attached a D5 patch that fixes it slightly differently than the D6 patch. I'll adapt the D6 patch if that is what will get committed.

For the record, here is one example (the one I encountered) that triggers buggy behavior:

drupal_bootstrap() can be called more than once including from within other bootstrap steps (e.g. statistics_exit() called during LATE_PAGE_CACHE calls drupal_bootstrap(DRUPAL_BOOTSTRAP_PATH)). As written, drupal_bootstrap() *always* shifts the next phase off the list and performs it.

This is wrong. Consider the scenario where you have two modules, A and B, both of which do what statistics.module does, and the page cache enabled:

function a_exit() { drupal_bootstrap(DRUPAL_BOOTSTRAP_PATH); }
function b_exit() { drupal_bootstrap(DRUPAL_BOOTSTRAP_PATH); }

1. index.php calls bootstrap(FULL).
2. bootstrap(FULL) goes as far as LATE_PAGE_CACHE. At this point, $phases still contains (PATH, FULL).
3. LATE_PAGE_CACHE finds a page cache hit, calls hook_boot(), outputs the cached page, and calls hook_exit().
4. a_exit() calls bootstrap(PATH).
5. bootstrap(PATH) shifts $phases, gets PATH, calls it, and returns.
6. b_exit() calls bootstrap(PATH).
5. bootstrap(PATH) shifts $phases, gets FULL, calls it (generating a "cannot set headers, output already started" error) and returns. Buzzz.

drumm’s picture

Status: Needs review » Fixed

Committed to 5.x.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.