It's seems that calling drupal_get_path_alias() from within hook_url_inbound_alter() effectively causes the cache path to break.
Perhaps resetting $cache['first_call'] to TRUE in redirect_url_inbound_alter() after calling drupal_get_path_alias() will fix the problem?
If you enable devel query log, you'll see that the cache_path is being updated on each request.
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | redirect-canonical_redirect-2048137-17.patch | 1.39 KB | heddn |
Comments
Comment #1
gielfeldt commentedNote: $cache refers to &drupal_static('drupal_lookup_path');
Comment #2
mfbI am seeing this too. I believe it happens on any page which is a path alias.
Since this appears to be dead code, the easiest fix would be to comment it out.
Comment #3
berdirYes, this is a critical performance issue. Found this out myself too.
Comment #4
pere orgaWouldn't be better to delete the code instead?
Comment #5
pere orgathere are no references to 'redirect_canonical' variable. Let's remove it. Changing the status to 'major' but feel free to change it if you feel strong about it.
Comment #6
mfbDeleting nonworking code sounds great to me :)
My only comment would be that if this variable used to exist in a previous version of the module, then an update script should delete it.
Comment #7
dave reidLet's comment out the code please instead of deleting.
Comment #8
gielfeldt commentedIn my experience, committing commented out/disabled code is usually a bad idea for several reasons. VCS is not a backup archive, and can it can be potentially problematic determining the correct history of the code.
Comment #9
dave reidThere is intent to make this code work. I, the maintainer, would rather it stay commented-out so I can move it and un-comment it, rather than me having to go back and find the hash where the code existed.
Comment #11
pere orgaIn that case, in patch #2 I would comment out this line too:
Comment #12
mfbpatch at #2 still applies
Comment #13
fabianx commentedCNW per #7, this won't be applied unless a proper patch is made per maintainer policy.
Comment #14
mfbCan you review the patch at #2?
Comment #16
fabianx commentedOh, I see what you mean. Woops.
RTBC for #2. I was confused.
Comment #17
azinck commentedGot bitten by this, too. Can confirm #2 works well.
Comment #18
heddnLeaving this as RTBCed, since #18 just comments out the wrapping conditional and adds a comment back to this issue.
Comment #19
azinck commentedWhile this is certainly code that should be removed/commented-out, the root of this problem is really an issue with core: #2400539: Path cache won't always contain the canonical alias, depending on order of calls to drupal_lookup_path()
Comment #21
dave reidOk very interesting that even those these API functions are available at this point, they cause data integrity. Strange. I'll keep this in mind for when I come back to looking at this code, so committing #18 for now.