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.
When declaring all used caches backends in settings.php like that:
$conf['cache_backends'][] = 'sites/all/modules/mongodb/mongodb_cache/mongodb_cache.inc';
$conf['cache_backends'][] = 'sites/all/modules/apc/drupal_apc_cache.inc';
$conf['cache_backends'][] = 'sites/all/modules/redis/redis.autoload.inc';
The code used in mongodb_cache.inc will be executed before the code used in other backends, in _drupal_bootstrap_page_cache:
foreach (variable_get('cache_backends', array()) as $include)
{
require_once DRUPAL_ROOT . '/' . $include;
}
Now the first lines of this inc file are:
if (!drupal_load('module', 'mongodb')) {
include_once dirname(__FILE__) . '/../mongodb.module';
}
- Why testing the module load before doing an include_once, why not simply use a require_once and let apc opcode optimize require_once calls?
- drupal_load() will use drupal_get_filename which will make a cache_get(), and this cache_get may be using a different storage engine (like Redis) which is not yet lodaed in the _drupal_bootstrap_page_cache foreach loop => Crash, and maybe infinite loop if mongodb_cache is use for that cache.
A simple fix is to comment the if (not the include).
Comments
Comment #1
regilero CreditAttribution: regilero commentedAttached patch is simply commenting the if.
Comment #3
regilero CreditAttribution: regilero commentedPrevious patch was a 7.x-1.x patch, this one is really a 7.x-1.0-rc1 patch
Comment #4
regilero CreditAttribution: regilero commentedComment #6
chx CreditAttribution: chx commentedI fixed this by changing the condition to function_exists('mongodb'). Thanks for the report and the patch.
Comment #7
regilero CreditAttribution: regilero commentedOk,
But why even making a function exists, the goal of require_once is to avoid re-doing the require if it's already done, so PHP will do that check for you, and even more APC could do that for you, certainly faster than the function_exists check when millions of functions are loaded in memory?
Comment #8
chx CreditAttribution: chx commentedOne Zend hash lookup is not particularly expensive while doing file I/O might be.
Comment #9
regilero CreditAttribution: regilero commentedA require_once() call on an already included file will not make any file IO, AFAIK. Especially if you have an opcode like APC.
Comment #10
chx CreditAttribution: chx commentedOK. You made me read up on this. You should know that require_once was really badly inefficient in the past. On the other hand this has been fixed as long as PHP 5.2.0. So there's no more reason to avoid it, I will revert mine and commit yours.
Comment #11
regilero CreditAttribution: regilero commentedNice,
For people using PHP prior du 5.2 they'll have to use a good opcode, but I prefer having cleaner code, simple is beautiful.
Comment #12
chx CreditAttribution: chx commentedThey can't use PHP before 5.2.4 with D7 anyways.
Comment #13
Rok Žlender CreditAttribution: Rok Žlender commentedRerolled the patch according to comment in #10
Comment #15
marcingy CreditAttribution: marcingy commentedLooks good but needs a re-roll obviously
Comment #16
chx CreditAttribution: chx commentedThanks everyone.