hook_boot() not invoked on uncached page views if cache mode is AGGRESSIVE
gpk - October 20, 2008 - 00:27
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
Looks like this was introduced when fixing #182675: Normal page caching disables hook_boot() and hook_exit().
#17 in that issue also points out that the loading of module.inc was moved. Since it's not needed for a page served from the cache when in aggressive mode, this patch also fixes that (=> should give minor performance improvment. Note that 5.x similarly has module.inc included just before http://api.drupal.org/api/function/bootstrap_invoke_all/5 in http://api.drupal.org/api/function/_drupal_cache_init/5).
| Attachment | Size |
|---|---|
| hook_boot_aggressive.patch | 1.01 KB |
| Testbed results | ||
|---|---|---|
| hook_boot_aggressive.patch | failed | Failed: Failed to apply patch. a href=http://testing.drupal.org/pifr/file/1/hook_boot_aggressive.patchDetailed results/a |
#1
Oh yes there's a patch here ;)
#2
The last submitted patch failed testing.
#3
I assume the test system doesn't like the CRs I seem to get running diff on Win?
Trying again..
Bug also present in 6.x BTW.
#4
#5
I can confirm this behavior. If cache is set aggressive and a user is logged in, hook_boot() will not fire but hook_exit() will. Note, this patch also skips loading the './includes/module.inc' if the page has been cached.
Patches attached for D6 and D7 revised from #3 to not change the comment about loading the page cache.
#6
@5: Thanks for reinstating the comment before the module.inc line, omitted in error. Very minor point but not sure why you've reverted the comment about loading the page cache?
#7
The last submitted patch failed testing.
#8
Sigh...that's the not last submitted patch.
#9
The last submitted patch failed testing.
#10
Comon testing bot...let's not fight. If anyone from testing.d.org reads, please see why the patch from #5 is not considered 'the last test' and is not being tested?
#11
The last submitted patch failed testing.
#12
-d6.patch shouldn't get tested. -d7.patch probably should, but I wonder if it's getting caught by the same regexp. Easy way to find out.
#13
#14
#12 now showing in the queue http://testing.drupal.org/pifr/node/1/323474. So maybe it did get caught by the regex.
#15
d.o experienced some database issues earlier today, and i'm guessing 323474-hook-boot-d7.patch in #5 got lost in the fray.
i've re-tested our regex, and it does allow -d7.patch extensions to be submitted for testing, so chalk this one up to a freak accident.
#16
The last submitted patch failed testing.
#17
Failed due to #74645: modify file_scan_directory to include a regex for the nomask.. Setting back to code needs review.
#18
Reposting patches (D7 had a small offset) so I can get a green light from testing.d.org and hopefully get this committed soon. It's an easy fix.
#19
Could we write a simpletest for this - enable aggressive caching, make sure cache is clear, module which implements hook_boot, watch everything blow up?
#20
Sure thing...let me get one written quick. Will repost patch with test.
#21
There we are! Simple little test included that passes 100% (and would have 1 failure without the patch). Note that the D6 patch is still valid in #18.
#22
Patch looks good, tests pass. Re-rolled only with extraneous phpdoc removed + a line added for the new class. No credit on commit please.
#23
I've committed this to CVS HEAD. I assume that the DRUPAL-6 patch should also go in still?
#24
@23: Yes, #18 still good for D6.
Attached followup patch corrects a garbled comment in the tests, plus a couple of other doc tweaks in the tests.. temporarily resetting version..
(@21: Cool, I was wondering how this could be tested.)
[update: patch removed]
#25
Proper patch this time..
#26
Doc cleanup committed to D7.
Moving back to D6 for consideration of #18.
#27
Patch in #18 still applies to latest 6.x and is ready to be committed.
#28
Committed to Drupal 6.x as well.
#29
Automatically closed -- issue fixed for two weeks with no activity.