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';
  }
  1. 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?
  2. 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).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

regilero’s picture

Attached patch is simply commenting the if.

Status: Needs review » Needs work
regilero’s picture

Previous patch was a 7.x-1.x patch, this one is really a 7.x-1.0-rc1 patch

regilero’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
chx’s picture

Status: Needs work » Fixed

I fixed this by changing the condition to function_exists('mongodb'). Thanks for the report and the patch.

regilero’s picture

Ok,

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?

chx’s picture

One Zend hash lookup is not particularly expensive while doing file I/O might be.

regilero’s picture

A require_once() call on an already included file will not make any file IO, AFAIK. Especially if you have an opcode like APC.

chx’s picture

Status: Fixed » Reviewed & tested by the community

OK. 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.

regilero’s picture

Nice,

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.

chx’s picture

They can't use PHP before 5.2.4 with D7 anyways.

Rok Žlender’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
481 bytes

Rerolled the patch according to comment in #10

Status: Needs review » Needs work

The last submitted patch, mongodb_cache_include_fix-1700108-13.patch, failed testing.

marcingy’s picture

Status: Needs work » Reviewed & tested by the community

Looks good but needs a re-roll obviously

chx’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.