SessionCacheContext calls getId() on null

It seems clear that the cli is not properly initializing the core request with a session, probably a cli issue.

However, it seems acceptable to put some safety catch into the \Drupal\Core\Cache\Context\SessionCacheContext. Any proper deeper core fix would be in symfony,

Here is the code that triggers an error:

  /**
   * {@inheritdoc}
   */
  public function getContext() {
    $sid = $this->requestStack->getCurrentRequest()->getSession()->getId();
    return Crypt::hashBase64($sid);
  }

suggestions?

reformatted the issue body

@guncha25 can you please make the method return some static predictable hash if the session $sid is not found.

Some predictable hash that is static across the request would be good.

Returned the Crypt::hashBase64(1) as predictable hash.

+++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php
@@ -22,8 +23,11 @@ public static function getLabel() {
+    return Crypt::hashBase64(1);

Returning a hash of a constant value seems wrong and could lead to an artificial sense of this being something secure. Why not just return 0 or 'none' here? If there is no session, then there is no session context?

Also I note that SessionCacheContext is used as a cache context, but doesn't implement \Drupal\Core\Cache\Context\CacheContextInterface - perhaps it should, otherwise it's missing methods might cause other fatals namely getCacheableMetadata. Perhaps that is a different issue though.

Please use $request->hasSession() to test for the presence of a session on the request.

Do we need a test?

+++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php
@@ -22,8 +23,18 @@ public static function getLabel() {
+    if ($this->requestStack->getCurrentRequest()->hasSession()) {
+      $sid = $this->requestStack->getCurrentRequest()->getSession()->getId();

we could store the current request in a local variable to simplify here

Do we need a test?

I think this is always a good indicator to add a test :)

+++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php
@@ -22,8 +23,18 @@ public static function getLabel() {
   public function getContext() {
-    $sid = $this->requestStack->getCurrentRequest()->getSession()->getId();
-    return Crypt::hashBase64($sid);
+    $request = $this->requestStack->getCurrentRequest();
+    if ($request->hasSession()) {
+      return Crypt::hashBase64($request->getSession()->getId());
+    }
+    return 'none';
+  }

Do we have an issue to actually hash the values in the session instead of just hashing the session ID?

Added CacheContextInterface as one of points here was to make cache handling more graceful.

Is this required to fix this particular problem?

I've tested the patch, but it doesn't solve the error in my case. Here is the error after applied the patch:

$ DRUSH_PHP=php70 drush ev "new \Drupal\om_1t_interface\IntegrationServices\TaskQueueRepairHistory;"
Error: Call to a member function getId() on null in Drupal\user\PrivateTempStore->getOwner() (line 210 of                                                   [error]
docroot/core/modules/user/src/PrivateTempStore.php) #0
docroot/core/modules/user/src/PrivateTempStore.php(200): Drupal\user\PrivateTempStore->getOwner()
#1 docroot/core/modules/user/src/PrivateTempStore.php(102): Drupal\user\PrivateTempStore->createkey('Token')
#2 docroot/modules/custom/om_1t_interface/src/Helper/UserSession.php(72):
Drupal\user\PrivateTempStore->get('Token')
#3 docroot/modules/custom/om_1t_interface/src/Helper/UserSession.php(134):
Drupal\om_1t_interface\Helper\UserSession::getPropertyValue('Token')
#4
docroot/modules/custom/om_1t_interface/src/IntegrationServices/FirstTouchServiceAbstract.php(73):
Drupal\om_1t_interface\Helper\UserSession::getToken()
#5
docroot/modules/custom/om_1t_interface/src/IntegrationServices/FirstTouchServiceAbstract.php(38):
Drupal\om_1t_interface\IntegrationServices\FirstTouchServiceAbstract->setSecurityHeader()
#6 docroot/modules/custom/om_1t_interface/src/IntegrationServices/Status.php(25):
Drupal\om_1t_interface\IntegrationServices\FirstTouchServiceAbstract->__construct('IdentityService', 'GetStatus')
#7 docroot/modules/custom/om_1t_interface/src/IntegrationServices/Status.php(46):
Drupal\om_1t_interface\IntegrationServices\Status->__construct()
#8 docroot/modules/custom/om_1t_interface/src/IntegrationServices/TaskServiceAbstract.php(36):
Drupal\om_1t_interface\IntegrationServices\Status::checkToken()
#9 vendor/drush/drush/commands/core/core.drush.inc(1168) : eval()'d code(1):
Drupal\om_1t_interface\IntegrationServices\TaskServiceAbstract->__construct()
#10 vendor/drush/drush/commands/core/core.drush.inc(1168): eval()
#11 vendor/drush/drush/includes/command.inc(422): drush_core_php_eval('new \\Drupal\\om_...')
#12 vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#13 vendor/drush/drush/includes/command.inc(199): drush_command('new \\Drupal\\om_...')
#14 vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#15 vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#16 vendor/drush/drush/drush.php(12): drush_main()
#17 {main}.
Error: Call to a member function getId() on null in docroot/core/modules/user/src/PrivateTempStore.php on line 210 #0 docroot/core/modules/user/src/PrivateTempStore.php(200): Drupal\user\PrivateTempStore->getOwner()
#1 docroot/core/modules/user/src/PrivateTempStore.php(102): Drupal\user\PrivateTempStore->createkey('Token')
#2 docroot/modules/custom/om_1t_interface/src/Helper/UserSession.php(72): Drupal\user\PrivateTempStore->get('Token')
#3 docroot/modules/custom/om_1t_interface/src/Helper/UserSession.php(134): Drupal\om_1t_interface\Helper\UserSession::getPropertyValue('Token')
#4 docroot/modules/custom/om_1t_interface/src/IntegrationServices/FirstTouchServiceAbstract.php(73): Drupal\om_1t_interface\Helper\UserSession::getToken()
#5 docroot/modules/custom/om_1t_interface/src/IntegrationServices/FirstTouchServiceAbstract.php(38): Drupal\om_1t_interface\IntegrationServices\FirstTouchServiceAbstract->setSecurityHeader()
#6 docroot/modules/custom/om_1t_interface/src/IntegrationServices/Status.php(25): Drupal\om_1t_interface\IntegrationServices\FirstTouchServiceAbstract->__construct('IdentityService', 'GetStatus')
#7 docroot/modules/custom/om_1t_interface/src/IntegrationServices/Status.php(46): Drupal\om_1t_interface\IntegrationServices\Status->__construct()
#8 docroot/modules/custom/om_1t_interface/src/IntegrationServices/TaskServiceAbstract.php(36): Drupal\om_1t_interface\IntegrationServices\Status::checkToken()
#9 vendor/drush/drush/commands/core/core.drush.inc(1168) : eval()'d code(1): Drupal\om_1t_interface\IntegrationServices\TaskServiceAbstract->__construct()
#10 vendor/drush/drush/commands/core/core.drush.inc(1168): eval()
#11 vendor/drush/drush/includes/command.inc(422): drush_core_php_eval('new \\Drupal\\om_...')
#12 vendor/drush/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#13 vendor/drush/drush/includes/command.inc(199): drush_command('new \\Drupal\\om_...')
#14 vendor/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#15 vendor/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#16 vendor/drush/drush/drush.php(12): drush_main()
#17 {main}
Error: Call to a member function getId() on null in Drupal\user\PrivateTempStore->getOwner() (line 210 of docroot/core/modules/user/src/PrivateTempStore.php).
Drush command terminated abnormally due to an unrecoverable error.                                                                                          [error]

Search API indexing wasn't updating for me during cron runs. After this patch everything ran once more.
Not sure if I should mark as reviewed due to comment #16.

Got this issue:

Error: Call to a member function getId() on null in Drupal\Core\Cache\Context\SessionCacheContext->getContext() (line 25 of                                                           [error]
/var/www/html/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php) #0 /var/www/html/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php(118):
Drupal\Core\Cache\Context\SessionCacheContext->getContext(NULL)
#1 /var/www/html/core/lib/Drupal/Core/Render/RenderCache.php(307): Drupal\Core\Cache\Context\CacheContextsManager->convertTokensToKeys(Array)
#2 /var/www/html/core/lib/Drupal/Core/Render/RenderCache.php(93): Drupal\Core\Render\RenderCache->createCacheID(Array)
#3 /var/www/html/core/lib/Drupal/Core/Render/PlaceholderingRenderCache.php(127): Drupal\Core\Render\RenderCache->set(Array, Array)
#4 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(525): Drupal\Core\Render\PlaceholderingRenderCache->set(Array, Array)
#5 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, true)
#6 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(151): Drupal\Core\Render\Renderer->render(Array, true)
#7 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(574): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#8 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(152): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#9 /var/www/html/core/modules/node/src/Plugin/Search/NodeSearch.php(480): Drupal\Core\Render\Renderer->renderPlain(Array)
#10 /var/www/html/core/modules/node/src/Plugin/Search/NodeSearch.php(452): Drupal\node\Plugin\Search\NodeSearch->indexNode(Object(Drupal\node\Entity\Node))
#11 /var/www/html/core/modules/search/search.module(195): Drupal\node\Plugin\Search\NodeSearch->updateIndex()
#12 [internal function]: search_cron()
#13 /var/www/html/core/lib/Drupal/Core/Extension/ModuleHandler.php(391): call_user_func_array('search_cron', Array)
#14 /var/www/html/core/lib/Drupal/Core/Cron.php(223): Drupal\Core\Extension\ModuleHandler->invoke('search', 'cron')
#15 /var/www/html/core/lib/Drupal/Core/Cron.php(122): Drupal\Core\Cron->invokeCronHandlers()
#16 /var/www/html/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#17 phar:///usr/bin/drush/commands/core/core.drush.inc(674): Drupal\Core\ProxyClass\Cron->run()
#18 phar:///usr/bin/drush/includes/command.inc(422): drush_core_cron()
#19 phar:///usr/bin/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#20 phar:///usr/bin/drush/includes/command.inc(199): drush_command()
#21 phar:///usr/bin/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#22 phar:///usr/bin/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#23 phar:///usr/bin/drush/includes/startup.inc(458): drush_main()
#24 phar:///usr/bin/drush/includes/startup.inc(365): drush_run_main(false, '/', 'Phar detected. ...')
#25 phar:///usr/bin/drush/drush(114): drush_startup(Array)
#26 /usr/bin/drush(10): require('phar:///usr/bin...')
#27 {main}.

Error: Call to a member function getId() on null in /var/www/html/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php on line 25 #0 /var/www/html/core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php(118): Drupal\Core\Cache\Context\SessionCacheContext->getContext(NULL)
#1 /var/www/html/core/lib/Drupal/Core/Render/RenderCache.php(307): Drupal\Core\Cache\Context\CacheContextsManager->convertTokensToKeys(Array)
#2 /var/www/html/core/lib/Drupal/Core/Render/RenderCache.php(93): Drupal\Core\Render\RenderCache->createCacheID(Array)
#3 /var/www/html/core/lib/Drupal/Core/Render/PlaceholderingRenderCache.php(127): Drupal\Core\Render\RenderCache->set(Array, Array)
#4 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(525): Drupal\Core\Render\PlaceholderingRenderCache->set(Array, Array)
#5 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(195): Drupal\Core\Render\Renderer->doRender(Array, true)
#6 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(151): Drupal\Core\Render\Renderer->render(Array, true)
#7 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(574): Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}()
#8 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(152): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#9 /var/www/html/core/modules/node/src/Plugin/Search/NodeSearch.php(480): Drupal\Core\Render\Renderer->renderPlain(Array)
#10 /var/www/html/core/modules/node/src/Plugin/Search/NodeSearch.php(452): Drupal\node\Plugin\Search\NodeSearch->indexNode(Object(Drupal\node\Entity\Node))
#11 /var/www/html/core/modules/search/search.module(195): Drupal\node\Plugin\Search\NodeSearch->updateIndex()
#12 [internal function]: search_cron()
#13 /var/www/html/core/lib/Drupal/Core/Extension/ModuleHandler.php(391): call_user_func_array('search_cron', Array)
#14 /var/www/html/core/lib/Drupal/Core/Cron.php(223): Drupal\Core\Extension\ModuleHandler->invoke('search', 'cron')
#15 /var/www/html/core/lib/Drupal/Core/Cron.php(122): Drupal\Core\Cron->invokeCronHandlers()
#16 /var/www/html/core/lib/Drupal/Core/ProxyClass/Cron.php(75): Drupal\Core\Cron->run()
#17 phar:///usr/bin/drush/commands/core/core.drush.inc(674): Drupal\Core\ProxyClass\Cron->run()
#18 phar:///usr/bin/drush/includes/command.inc(422): drush_core_cron()
#19 phar:///usr/bin/drush/includes/command.inc(231): _drush_invoke_hooks(Array, Array)
#20 phar:///usr/bin/drush/includes/command.inc(199): drush_command()
#21 phar:///usr/bin/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#22 phar:///usr/bin/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#23 phar:///usr/bin/drush/includes/startup.inc(458): drush_main()
#24 phar:///usr/bin/drush/includes/startup.inc(365): drush_run_main(false, '/', 'Phar detected. ...')
#25 phar:///usr/bin/drush/drush(114): drush_startup(Array)
#26 /usr/bin/drush(10): require('phar:///usr/bin...')
#27 {main}
Error: Call to a member function getId() on null in Drupal\Core\Cache\Context\SessionCacheContext->getContext() (line 25 of /var/www/html/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php).

This patch helped me.

Drupal core 8.3.5

I had this issue with a sub requests and the patch fixed it. Thx

+++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php
@@ -22,8 +23,18 @@ public static function getLabel() {
+    if ($request->hasSession()) {
+      return Crypt::hashBase64($request->getSession()->getId());

what if session is not started?

Like @moonray in #18, I ran into this when running search_cron(). I was running cron via drush, so there was no session. I am not sure why only one node out of 2700 or so caused a problem, but this patch fixed it for me.

The problem in #16 is from a different part of the code. This issue is not supposed to solve all bugs in Drupal core that lead to Error: Call to a member function getId() on null.

Outstanding issues:

1. In #9, @larowlan suggested implementing CacheContextInterface (with the caveat "Perhaps that is a different issue though"). The current patch implements that suggestion, but in #14 @dawehner questioned that decision.
2. In #22, @andypost asks, "what if session is not started?"

I am not qualified to decide whether (1) is a good idea.

For (2), I see the following in Request.php, although I am still using Drupal 8.3 so Symfony 2.8 or whatever ... this may change with Drupal 8.4 and Symfony 3.2 or whatever.

    public function getSession()
    {
        return $this->session;
    }
    public function hasSession()
    {
        return null !== $this->session;
    }

So I think it is pretty clear (for this version of Symfony) that if hasSession() returns true, then getSession() will not return null.

Update: I checked with Drupal 8.4.x, and the code above has not changed in symfony/http-foundation v3.2.8.

I did not review the test. I am adding a test-only version of @gunch25's patch from #13. If this patch fails, then it should be considered a success for the real patch.

I am re-testing with Drupal 8.3.x as a sanity check. (Maybe just my sanity.) This patch will need a re-roll for 8.4. That is a novice task.

Re-rolled.

@Jo Fitzgerald: It looks as though you re-rolled the test-only patch, so we expect a failure. (I checked the failure on my test-only patch from #24, and it failed in the expected way.

We still need re-roll of the patch from #13.

@benjifisher in my comment I mean that session always exists in core & you need additionally to check for \Symfony\Component\HttpFoundation\Session\SessionInterface::isStarted()

@andypost I am afraid I do not understand what you mean. According to the error message (before the patch) getSession() is returning null. So what exactly do you mean by "session always exists in core"?

Remember, we only see this error when using CLI tools (drupal console and drush).

I am pretty sure that the patch works: if getSession() returns null, then hasSession() returns false, so this should work:

+    if ($request->hasSession()) {
+      return Crypt::hashBase64($request->getSession()->getId());
+    }
+    return 'none';

My only quibble is that I would turn it around:

+    if (!$request->hasSession()) {
+      return 'none';
+    }
+    return Crypt::hashBase64($request->getSession()->getId());

or maybe even a ternary

+    return $request->hasSession() ? Crypt::hashBase64($request->getSession()->getId()) : 'none';

Really sorry, I must've been half asleep when I did the re-roll. Here is a re-rolled version of the patch in #13.

I tried to create an interdiff for the patches in #13 and #31, and it came up empty. I guess that is the expected behavior when the only changes are in the context lines of the patch (replacing \PHPUnit_Framework_TestCase with UnitTestCase).

Based on my review in #23 above, I am marking this RTBC.

@andypost: I noticed the following code in core/lib/Drupal/Core/StackMiddleware/Session.php:

  public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) {
    if ($type === self::MASTER_REQUEST && PHP_SAPI !== 'cli') {
      $session = $this->container->get($this->sessionServiceName);
      $session->start();
      $request->setSession($session);
    }
    // ...
    return $result;
  }

So in this case, the session (on the Request object) is started and set at the same time, and neither happens when Drupal is run from the CLI. Possibly there are other code paths where the session is set but not started.

+++ b/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php
@@ -35,21 +42,22 @@ class SessionCacheContextTest extends UnitTestCase {
-    $this->cacheContext = new SessionCacheContext($this->requestStack);

I don't think we should be storing and using $this->cacheContext on the test class anymore. Each test is just creating it's own cache context object so it can just be a local variable ie $cache_context - which is nice because it is the thing under test.

@alexpott So you want to remove the $cacheContext property from this test class, remove the line that sets it in the setUp() method, and then do

    $cacheContext = new SessionCacheContext($this->requestStack);

in each test method, and replace $this->cacheContext with $cacheContext in those methods?

If I have it right, then this looks like a novice task.

+++ b/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php
@@ -35,21 +42,22 @@ class SessionCacheContextTest extends UnitTestCase {
+    $this->cacheContext = new SessionCacheContext($this->requestStack);

@@ -65,6 +73,9 @@ public function testSameContextForSameSession() {
+    $this->cacheContext = new SessionCacheContext($this->requestStack);

@@ -83,4 +94,15 @@ public function testDifferentContextForDifferentSession() {
+    $this->cacheContext = new SessionCacheContext($this->requestStack);

It is already happening in every test. The ::setUp() used to set it but now it does not because of changes made by this patch. Therefore the change is in scope.

Sorry, I misunderstood. I guess what I said was half right, or maybe 1/3 right.

1. Remove the $cacheContext property from this test class.
2. Replace $this->cacheContext with a local variable $cache_context in each test method.

Here is an updated patch and interdiff as per comment #37

@Dinesh18:

When you provide a patch, please also upload an interdiff comparing it to the previous version. (If you add a new patch, please compare it to the one in #31, not your patch in #38.) Instructions are here: Creating an interdiff.

@@ -32,31 +39,31 @@ class SessionCacheContextTest extends UnitTestCase {
    *
    * @var \Drupal\Core\Cache\Context\SessionCacheContext
    */
-  protected $cacheContext;

When you remove the property, you should also remove its code comment. After applying your patch, I see this in the code:

  /**
   * The session cache context.
   *
   * @var \Drupal\Core\Cache\Context\SessionCacheContext
   */

  public function setUp() {
    // ...
  }

I am surprised that there is no doc block for the setUp(). I would expect the standard {@inheritdoc}. Please leave it as is for this issue: fixing it would be out of scope, and it might even be correct as it is. There are already some exceptions for documenting PHPUnit classes, although leaving off the method's doc block is not listed at https://www.drupal.org/node/2116043. (See "Required PHPDoc metadata for test discoverability" near the bottom of the page.)

@benjifisher, I have provided the interdiff.txt in #38

@Dinesh18 Thanks.

@anya_m This looks like one of your first patches. Welcome! As I said in #39, I think an interdiff against the patch in #31 will be more useful. I will attach one.

I have reviewed the patch, and I think it does what @alexpott requested. I am marking this issue RTBC, and I will be a little embarrassed if the testbot has a different opinion.

+++ b/core/lib/Drupal/Core/Cache/Context/SessionCacheContext.php
@@ -3,13 +3,14 @@
+class SessionCacheContext extends RequestStackCacheContextBase implements CacheContextInterface {
 

@@ -22,8 +23,18 @@ public static function getLabel() {
+  /**
+   * {@inheritdoc}
+   */
+  public function getCacheableMetadata() {
+    return new CacheableMetadata();
   }

This is not actually part of this issue. As @larowlan noted in #9

Also I note that SessionCacheContext is used as a cache context, but doesn't implement \Drupal\Core\Cache\Context\CacheContextInterface - perhaps it should, otherwise it's missing methods might cause other fatals namely getCacheableMetadata. Perhaps that is a different issue though.

I think the addition of the interface should go in a separate issue. Afaics from the code because the ID does not contain a "." or ":" we'll never call getCacheableMetadata(). If this change doesn't have the interface addition then it is a bugfix that can get resolved in 8.4.x as well as 8.5.x. Which is good because it is fixing a problem that people are facing.

I added the follow-up issue #2915594: SessionCacheContext class should implement CacheContextInterface.

@alexpott, I see that you removed the Novice tag. I am happy to supply a new patch, although I prefer not to review my own patches.

New patch attached, and moving back to 8.4.x, according to @alexpott's comment in #44.

If you reverse the interdiff, it might be good enough for #2915594: SessionCacheContext class should implement CacheContextInterface.

SessionCacheContextTest is passing locally. It would be embarrassing if it failed.

Follow-up filed & all feedback addressed

Thanks, @andypost!

Looking at the list of files you hid, I realize that we should have an updated test-only patch. It is attached.

We expect this patch to fail on SessionCacheContextTest. If it does, then we can revert the testbot's NW to RTBC.

Yay, the test-only patch failed in the expected way. Back to RTBC as in #47.

+++ b/core/tests/Drupal/Tests/Core/Cache/Context/SessionCacheContextTest.php
@@ -75,12 +79,22 @@ public function testDifferentContextForDifferentSession() {
+   * @runInSeparateProcess

I'm really sorry I should have noticed this before. I'm don't see why this is necessary. As far as I can see there are no globals being affected here. If it is necessary there should be some documentation as to why.

@benjifisher thanks for uploading a test-only patch - if you do that you also need to upload the rtbc patch too so that is last on the files list. This means that the rtbc re-test job won't set the issue back to needs works unnecessarily.

@alexpott: OK, next time I will be less considerate of the testbot. I hate to give the poor bot extra work.

The @runInSeparateProcess was added by @guncha in #13, the first patch on this issue that touched the test class. Maybe I should get to know the testing system better before I next mark an issue RTBC.

I have added a new patch, a test-only version (in the correct order) and an interdiff.

rtbc again)

Ticking reviewer credit boxes for reviewers that have influenced the direction of the patch.

Committed and pushed 0c0c63e9af to 8.5.x and 3c7f090ea2 to 8.4.x. Thanks!

Yay! Now my cron jobs will work without a custom patch.

I just posted a patch on the follow-up issue #2915594: SessionCacheContext class should implement CacheContextInterface. All I did was reverse interdiff-39-46.txt from this issue.