Incorrect expiration in APCUBackend

And *how* exactly can $expire be a string?

A failing regression test would answer #2.

Just looking through core calls to set, almost everything uses a constant. This snippet from ChainedFastBackend looks like a likely culprit:

      foreach ($this->consistentBackend->getMultiple($cids, $allow_invalid) as $item) {
        $cache[$item->cid] = $item;
        // Don't write the cache tags to the fast backend as any cache tag
        // invalidation results in an invalidation of the whole fast backend.
        $this->fastBackend->set($item->cid, $item->data, $item->expire);
      }

Yes. It's coming from the database (for the default mysql backend). Everything that comes from the database is a string.

We can also type cast to integer there, but we've had plenty of bugs already due to incorrect type safe checks when e.g. values from the database were involved.

Also, the code is still wrong. We're using an absolute expiration date as a ttl.

We also have #2272685: Incorrect handling of invalid cache items in apcu backend, maybe we should just do that, but I'm also not sure that completely removing ttl is the right thing to do. We've had similar discussions in the redis backend decided to use ttl where possible and consider allow_invalid to apply mostly to cache tags.

I tired to reproduce it in a test but well, couldn't really.

+++ b/core/lib/Drupal/Core/Cache/ApcuBackend.php
@@ -178,14 +178,8 @@ public function set($cid, $data, $expire = CacheBackendInterface::CACHE_PERMANEN
+    // Expiration is handled by our own prepareItem(), not APC.

Nit: s/APC/APCu/

I'd like to see this get in during RC, so adding the tag so the core committers can discuss it.

Discussed briefly with alexpott and we also think this is a good rc target. Tagging.

I'm mulling this issue, but I think all the things that I'm mulling are off-topic, so will probably commit a bit later today.

I think we should test this behaviour in Drupal\system\Tests\Cache\ApcuBackendUnitTest so that we don't add it back again thinking it is missing.

Discussed with alexpott about testing this.

Using "apc_cache_info('user')", we can get a list of cached entries and their ttl. So we just need to extend a test method there and check that entry.

Note that we need to get the correct internal name, see how the factory builds the site prefix that is then combined with the bin.

Fixes the Nit in #10

INterdiff was wrong sorry,

Fixed the issue summary

Committed/pushed to 8.0.x, thanks!