Race condition with drupal_goto()

Patch for 4.7.7.

Flagging it for D6 instead so it'll get attention.

I'm not sure if it can be considered "race condition" or "expected behaviour", as drupal_goto is expected to fresh going the requested end-point, and you may return from the submit with an end-point of the form, and messages will be displayed.

So goin to the destination using the return of the form is a better way than using drupal_goto.. I guess that was the purpose of introducing the form return value.

Multiple calls to register_shutdown_function() can be made, and each will be called in the same order as they were registered. If you call exit() within one registered shutdown function, processing will stop completely and no other registered shutdown functions will be called.

session.inc registers session_write_close first, this patch gets the redirect second. The D5 patch in the original issue applies cleanly to D6 as well.

Thanks, committed to D6.

Uh, patch is ready for D5.

This is causing a white screen when drupal_goto() is called under PHP 4.4.7. http://drupal.org/node/180644

yes, specifically because of http://www.php.net/manual/en/function.register-shutdown-function.php :

Note: Shutdown function is called during the script shutdown so headers are always already sent.

Watchdog is reporting:

Cannot modify header information - headers already sent in Unknown on line 0.

And as the PHP docs say, you can’t modify the headers in shutdown function. Looks like we need to revert this change.

Also, here’s a suggested solution to the initial problem: Modify drupal_goto() to call session_write_close() before a call to header("Location: ..."). The subsequent, extraneous call to session_write_close() inside the shutdown function shouldn’t hurt anything.

The previous patch doesn't REVERSE cleanly, so here’s an updated patch that reverses and adds session_write_close() before drupal_goto(). I can split it into two patches if you like.

I’ve verified that no PHP errors, PHP warnings or Drupal watchdog messages are caused by running session_write_close() twice.

However, since I’m not experiencing the race condition, I can only say "I know this will fix the problem, but I haven’t tested it."

1. "We need all session data to be written to the database before the header is sent to the browser."

For future reference, I think we'll want to extend the PHPdoc with one line. That line should mention why this is required.

2. "... REDIRECT status code to the http daemon"

REDIRECT should be redirect and http should be HTTP.

3. The documentation lines wrap at inconsistent lengths.

Implemented Dries' notes.

Added PHPDoc note and straightened out comments (which were originally taken directly from the old code).

#12 or #13 is ready to go, choose the one you like more.

I confirm that drupal_goto() gives just blank screen and no redirect (php 4.4.4). I tested #13 patch, and it works fine.

I consider this as very critical bug, because now two subsequent 6.x-dev tarballs are horribly broken (every single form submission I tested ends up with white page - drupal_goto() is widely used function), so currently it makes other patches' (or any D6) testing a pain, and update.php (batch processing) doesn't work at all.

Committed to CVS HEAD. Thanks.

The call to session_write_close() is a great way to solve the problem. Nice work everyone.

Just looked at the comments in the patch and "addition" has a typo.

Nitpicking...

Revised nitpicking...

Even more nitpicking... thanks to dmitri for reviewing

Final version. Thanks to all nitpickers who reviewed this! :-D

Wow, that was a huge docs cleanup patch. I read through the improvements and all seemed to be fine, so committed. Please open new issues with such huge docs updates next time. It helps keep different issues separated. Thanks.

Re-rolled for 5.x. I didn’t include all the doc changes, just the bare minimum to fix this issue.

Committed to 5.x.