After upgrading to the latest dev version sites stopped loading.
The reason is the infinite loop in _rules_discover_module():

  ...
  $file_path = str_replace(DRUPAL_ROOT . '/', '', $reflection->getFileName());
  while (!isset($paths[$file_path]) && strpos($file_path, DIRECTORY_SEPARATOR) !== FALSE) {
    $file_path = dirname($file_path);
  }

Link: http://drupalcode.org/project/rules.git/blob/HEAD:/rules.module#l290

Halting conditions triggers when for example you have a Rules module directory as a symbolic link. In this case $reflection->getFileName() returns real path, and str_replace() doesn't replaces anything, and then the loop body $file_path = dirname($file_path); reaches "/" path and gets into inftinite iteration.

This should be fixed to
1) support symbolic links
2) not produce halting problem

Comments

OnkelTem’s picture

StatusFileSize
new152.91 KB

My symlinking story:

asd

fago’s picture

wow :-)

So, could you check what's the value of

  $file_path = str_replace(DRUPAL_ROOT . '/', '', $reflection->getFileName());

and
drupal_get_path('module', 'rules')
is for you?

At least it looks like the bail-out condition strpos($file_path, DIRECTORY_SEPARATOR) !== FALSE does not work, so we need to come up with something better here. e.g., just remove any slashes from the beginning?

OnkelTem’s picture

For the initial conditions when:

  1. Rules module is located in
    /var/www/lib/d7_dev/extensions/modules/rules/rules.module
  2. while having all modules available via the link:
    /var/www/sites/example.com/sites/~all -> /var/www/lib/d7_dev/extensions/
  3. and $class been passed to the function is 'RulesNodeConditionType'
  4. and DRUPAL_ROOT equals to /var/www/sites/example.com

we get:

  1. this entry in the $paths array:
    'sites/all/modules/rules' => 'rules'
  2. $reflection->getFileName() returns:
    /var/www/lib/d7_dev/extensions/modules/rules/modules/node.eval.inc
  3. $file_path = str_replace(DRUPAL_ROOT . '/', '', $reflection->getFileName()); returns just the same:
    /var/www/lib/d7_dev/extensions/modules/rules/modules/node.eval.inc
  4. And drupal_get_path('module', 'rules') is obviously:
    sites/all/modules/rules
OnkelTem’s picture

fago, I personally don't see any simple solution. Basically the task is in restoring original file structure for any files tree containing symlinks.
I could have for example:
/var/www/sites/example.com/sites/all/modules/rules as a real dir, while having:
/var/www/sites/example.com/sites/all/modules/rules/modules/@node.eval.inc as a symlink to another location, which is also a symlink to one more location, which is finally the symlink the Moon.
So you have to walk through the files, down (as a $class could be defined anywhere), testing is it a link, resolving each, recursively until you real the real file, and then walk up, finding it's place in the hierarchy of modules.

How to disable this part and rollback to the previous behavior?

OnkelTem’s picture

Can't we simply use {registry} table?

function _rules_discover_module($class) {
  // Just looking up in the registry
  $classes = &drupal_static(__FUNCTION__);
  if (!isset($classes[$class])) {
    $result = db_query('SELECT module FROM {registry} WHERE name = :class', array(':class' => $class));
    $classes[$class] = ($record = $result->fetchAssoc()) ? $record['module'] : FALSE;
  }
  return $classes[$class];
}
socialnicheguru’s picture

I tried the above change but my system did hang.

I had to revert to a previous version of rules.

I am using symlinks to link needed modules from a central directory

das-peter’s picture

StatusFileSize
new584 bytes

Another evil case: Windows
Because the directory separator is a back-slash rather than a slash the str_replace() function can't clean the file path either.
Attached patch changes . '/' to . DIRECTORY_SEPARATOR which will provide the OS specific directory separator.
Also there's this issue that looks related: #2060713: Maximum time exceeded: internal loop in _rules_discover_module if called with a class whose path cannot be found

plach’s picture

Status: Active » Needs review
StatusFileSize
new1.17 KB

What about this? It should correctly handle absolute paths, windows separators and avoid infinite loops.

das-peter’s picture

I like the approach and I think that should solve the issue. However, I just did a visual review for now.

+++ b/rules.module
@@ -284,13 +284,19 @@ function _rules_discover_module($class) {
+  $path = DIRECTORY_SEPARATOR != '/' ? str_replace(DIRECTORY_SEPARATOR, '/', $path) : $path;

Not sure if we need the condition here. Without I'd say it's more readable but it could affect the performance,?

plach’s picture

Well, it was just micro-optimization, I guess we can skip it if we prefer readability :)

stefan.korn’s picture

For me the patch from #8 works. I had the problem (Max execution time exceeded in rules.module) after transfering a drupal instance from Linux to Windows.

das-peter’s picture

Status: Needs review » Reviewed & tested by the community

I'm bold enough to say RTBC then as of #11.

fago’s picture

Status: Reviewed & tested by the community » Fixed

Thanks folks - committed!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

asd as always