Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
First requests to the httpKernel will cache request properly but the following requests handled by the same httpKernel instance will return the same response that was cached for the first request.
Proposed resolution
Add check for the comparison of cached cid and current cid in the getCacheId() method of the Drupal\page_cache\StackMiddleware\PageCache class.
Remaining tasks
-
User interface changes
-
API changes
-
Data model changes
-
Release notes snippet
-
Comment | File | Size | Author |
---|
Issue fork drupal-3050383
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
volegerComment #3
volegerComment #5
sames CreditAttribution: sames commentedThis problem persists in Drupal 8.7.0. I am glad I found the patch as it fixes the problem for me. Most people affected are probably using the subrequests contrib module.
Multiple workarounds to be found here:
https://www.drupal.org/project/subrequests/issues/3049395
Comment #6
waspper CreditAttribution: waspper as a volunteer and at Skilld commentedI've experienced same issue. Proposed patch seems to solve issue.
Comment #8
jhedstromThis will need tests, and to address the failing tests from the patch above.
Comment #10
mxr576Comment #11
anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commentedSlightly update the condition. Checking with 9.2.x
Comment #14
Lukas von BlarerI am also coming having this issue: #3061907: Page Cache duplicating responses for subrequests . But in my case dynamic_page_cache was causing this issue. Do we have the same bug there as well? Is there an issue regarding that already?
Comment #19
phenaproximaHiding patches in favor of the merge request.
Comment #20
phenaproximaThe only situation in which I've encountered this problem is in the Subrequests contrib module. See #3049395: Page Cache causes different subrequests to return the same responses.
In general, you wouldn't run into this problem, but Subrequests disguises its subrequests as master requests, specifically in order to allow page caching to kick in. Given that the Subrequests module's purpose is to be a driver for decoupled applications, that makes sense. In theory, Subrequests should just mark its subrequests as subrequests...but then page caching ceases to work for it, and its performance suffers dramatically.
By storing the page cache ID for a given request as an attribute of the request object, we can improve the situation for everyone, non-disruptively. However, I'd consider it a feature because core, from its perspective, is working as intended, and Subrequests can also work around the issue by subclassing the PageCache middleware.
Therefore, I'm re-categorizing this as a feature request and a contrib soft blocker.
Comment #22
volegerMR needs to rebase against 10.1.x branch
Comment #23
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #24
voleger@Bhanu951 please revert the merge commit, and do rebase the forked branch over 10.1.x. branch. Merge request has to show only a single commit `Store the page cache ID on the request` as a difference between branch 10.1.x
Comment #26
realityloop@volegar I've cleaned this branch up to 10.1.x and recommitted the change with attribution to yourself and phenaproxima
Comment #27
catchLeft a couple of comments on the MR.
Comment #28
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedComment #30
mglamanI think this should be using an SqlObjectStorage object for storing cache IDs per-request instead of request attributes. This avoids page_cache from manipulating the request. Just like \Drupal\Core\Path\CurrentPathStack
Copy of some work I'm adding to
It doesn't matter what kind of request it is then. And may help solve issues if running Drupal in an event loop runtime.
See https://git.drupalcode.org/project/subrequests/-/merge_requests/9/diffs?... for an example.
Comment #31
bradjones1Comment #34
mglamanI've created a new MR https://git.drupalcode.org/project/drupal/-/merge_requests/5745 that allows multiple main requests using the SplObjectStorage pattern, and a test. First commit showed test failure, second commit includes and shows the fix.
Comment #35
mglamanI tried to retitle to something more specific to the problem.
Comment #36
smustgrave CreditAttribution: smustgrave at Mobomo commentedAppears to be 1 open thread.
Comment #37
mglamanHow should we handle the open thread. I don't agree with @catch that we need a comment about why we're supporting multiple cache IDs, when now PageCache is acting as a per-request reverse proxy instead of main-request-only reverse proxy.