APCu detection erroneously succeeds if apc.enabled is "0"

This can be active.

apcu is the extension and PHP 5.4 is no longer supported.

Lets change all 'apc' to 'apcu' in the code and get rid of the emulation layer.

Well, since no one did it in over 1 year, here is a first patch.

Nice start,

There is also checks for apc_fetch and apc_clear, which need to be converted.

I didn't find any in 8.1 core. Where do you see them ? The only apc_* calls I get on a fresh install are below vendor/.

Indeed, we did the majority of this already in #2617568: Rename apc_* functions with apcu_*. I forgot about that.

Lets get the remainder in.

This needs work, because this issue is still referenced by other @todo's as dawehner found out in #2473179: Use FileCache for config storage.

Lets remove them all.

These are the instances of relevance that I'm finding that mention apc without mentioning apcu:

core/modules/file/file.install:107:    elseif ($implementation == 'apc') {
core/modules/file/file.module:932:    elseif (extension_loaded('apc') && ini_get('apc.rfc1867')) {
core/modules/file/file.module:933:      $implementation = 'apc';
core/modules/file/src/Controller/FileWidgetAjaxController.php:35:    elseif ($implementation == 'apc') {
core/modules/file/src/Element/ManagedFile.php:284:      elseif ($implementation == 'apc') {
core/tests/Drupal/KernelTests/Core/Cache/ApcuBackendTest.php:32:      if (PHP_SAPI === 'cli' && !ini_get('apc.enable_cli')) {

However, I don't know enough about this to know if this is anything that needs to change or not, but hopefully this helps.

I've also attached the full grep list:

Today I ran into a really strange issue. While debugging I recognized that the ApcClassLoader is used in an environment where apc is disabled!
The problem is caused by the way Drupal detects the availability of APCu. At the moment it's done like this:

if (function_exists('apcu_fetch')) {
  $loader = new ApcClassLoader($prefix, $this->classLoader);
}

I guess we use function_exists here as a workaround for the transition from APC to APCu. But even if we use extension_loaded('apcu') instead the implementation isn't reliable!

The APCu extension could be loaded but be disabled! In this case function_exists('apcu_fetch') and extension_loaded('apcu') will nevertheless both return TRUE.
It's important to check the ini settings as well. That's especially important if an extension is statically compiled into PHP.

For example this is a valid situation you may face:

var_dump(extension_loaded('apcu')); // boolean true
var_dump(function_exists('apcu_fetch')); // boolean true
var_dump(ini_get('apc.enabled')); // string '0'

So the correct detection should look like this:

if (extension_loaded('apcu') && ini_get('apc.enabled') && (ini_get('apc.enable_cli') || !drupal_is_cli())) {
  $loader = new ApcClassLoader($prefix, $this->classLoader);
}

I consider this a critical bug because Drupal must not ignore the configuration of it's environment (even if PHP behaves really stupid here).
Especially in a shared environment where APCu is available in the webserver as module but disabled for some applications due to memory limits or whatever.

I think http://php.net/manual/de/apcu.configuration.php#ini.apcu.enabled documents quite clearly that #15 is correct:

apc.enabled can be set to 0 to disable APC. This is primarily useful when APC is statically compiled into PHP, since there is no other way to disable it [...].

BTW symfony has the same issue :-(

Composer does it better but still not perfect:

    public function setApcuPrefix($apcuPrefix)
    {
        $this->apcuPrefix = function_exists('apcu_fetch') && ini_get('apc.enabled') ? $apcuPrefix : null;
    }

Here's a patch against 8.4.x first.

From IRC:
alexpott: well 8.2.x has had it's last release unless there is a super ciritical
mkalkbrenner: I assume it only hits a few people. But when it hits it's critical. Due to the fact that it already exists since 8.0 I think 8.3 is ok.
alexpott: that is likely to be the consensus

The detection doesn't verify the ini settings like apc.enabled. In some environments this leads to an usage of APCu even if it's disabled which might lead to any kind of problems

What are the problems? - "might" does not lead to this being critical. The issue summary needs to be updated to describe any real issues faced.

+++ b/core/modules/file/file.module
@@ -929,7 +929,7 @@ function file_progress_implementation() {
-    elseif (version_compare(PHP_VERSION, '7', '<') && extension_loaded('apc') && ini_get('apc.rfc1867')) {
+    elseif (extension_loaded('apcu') && ini_get('apc.enabled') && (ini_get('apc.enable_cli') || 'cli' !== php_sapi_name()) && ini_get('apc.rfc1867')) {

This change is not right - the issue that change this needs to be read and completely understood before altering this.

Also there have been a couple of changes to how the APC autoloader works in 8.2.6. See #2843828: \Drupal\Core\DrupalKernel::initializeSettings() can result in moving the autoloader position and #2840596: Update Symfony components to ~2.8.16. @mkalkbrenner can you confirm whether or not the problems were experienced on the latest stable version of D8 and also it would be great to know exactly what you experienced.

I need to to some more research which exact combination of environment setting and core version caused our trouble.
Due to the fact that the ApcClassloader is supposed to fall back to the standard class loader I decrease the priority.

Nevertheless I think that wrongly detecting the availability of APCu and relying on fallbacks in the class loader and the chained cache backend isn't the best architecture if the detection could be implemented correctly.
I assume the additional check of ini settings causes less overhead than a decorator that calls apc_fetch and then uses a fall back ;-)

Just uploading the patch for 8.2.5 core.

@alexpott, @catch, @cottser, @xjm and I considered this issue at a recent triage meeting and decided to postpone it based on #26 as more information is needed on the impact. If it can be shown to be reproducible in a realistic scenario, it will be triaged major priority.

Tried to reroll patch from #18 against 8.9.x. Since it was posted, the relevant codevae has changed though. #3054315: Sort out ApcuBackendTest substantially reworked ApcuBackendTest, removing getRequirements alltogether. Though some assumptions of the current patch may be cut out, but I left it as it is, not reverting (at least partially) commit from #3054315: Sort out ApcuBackendTest. Another relevant issue is #3053363: Remove support for PHP 5 in Drupal 8.8, where apc support was removed from file_progress implementation. But as the current patch should support PHP 7 as well, I restored it the reroll.

Thanks for the patch!

IIRC apcu_fetch will silently fail if the backend is disabled.

That is no real problem (and would even lead to bugs in chained fast if changed) as it’s acting as a NULL backend.

ChainedFast would have a bug if this was changed due to it needing the same configuration for cli and web - if the fast backend was only found for web, but not for cli a drush command would fail to clear the cache (as it would not update the timestamp in the DB),

As no one demonstrated a real bug with APCu, I will close this as works as designed.

Thanks again for your patch that brought that issue up again on my radar.

Actually back to active to remove the commented lines.

As no one demonstrated a real bug with APCu, I will close this as works as designed.

I described the "problem" in #15:

If you want to turn off APCu by setting apc.enabled = 0, you can't! That's a bug.

As soon as you disable it, it should act as a NULL cache backend.

If there is a bug, we need tests / steps to reproduce it.

The APCu extension could be loaded but be disabled! In this case function_exists('apcu_fetch') and extension_loaded('apcu') will nevertheless both return TRUE.

Can you point us to the code where this combination results in a NULL cache backend?

Correct me if I am wrong, but if apcu.enable == 0, then as far as I know the following should happen:

apcu_fetch($cid) == FALSE
apcu_store($cid, $data) == FALSE (IIRC)

It behaves like an always empty cache.

So what side effect / bug does that behavior cause? Trying to understand.

I have tried to do this in settings.php :

ini_set('apc.enabled', 0);
$aux = apcu_store('hola', 'mundo');
$aux = apcu_fetch('hola');

And it still saves the values into the cache !, so I guess it is impossible to disable APCu using "ini_set" when using the non-static PHP extension ??

I don't really understand why ini_set does not work, any help would be appreciated :)

You cannot disable APCu with ini_set because the "enabled" setting is a PHP_INI_SYSTEM value.