Today, I've been encountering a problem with my site - it crashes on every page load (sometimes with a PHP out of memory error). After some debugging, I was able to reproduce it from a minimal installation:

1. Install from the minimal profile
2. Enable the bean and bean admin UI module (and it's dependencies)
3. Add a block type
4. Add a block
5. Add this block to the blocks for the theme (used Bartik, footer region)
6. Verify that you can indeed see this Bean on all pages.
7. Enable the title module
8. Watch your site have a heart attack whenever Bartik is used (but not the admin pages)

I'm going to continue to investigate to see if I can narrow down the problem further, but it looks like something in the title module is causing the bean rendering process to use up all the PHP memory and then crash the site (and occasionally my server). Given the timing of this (it was working perfectly fine on builds done last week), I suspect this is something that was changed in title with the alpha7 release.

Anyone have any thoughts what might be causing this?

Update: Confirmed that reverting to alpha5 fixes this problem.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ottawadeveloper’s picture

I've narrowed down what's happening but I have no idea who should be fixing it or how:

The title patch added this line (title.module, line 124 about in title_entity_label). Title has changed the label callback of the module to title_entity_label, just for reference, and stored the original callback in the label fallback.

  elseif (isset($entity_info['label fallback']['title']) && function_exists($entity_info['label fallback']['title'])) {
    return $entity_info['label fallback']['title']($entity, $type, $langcode);
  }

This is called for the bean entity as we are not replacing the title field. For bean the original value is entity_class_label.

This triggers a call to the label() method of the Bean object (which inherit's it's definition from the Entity class). The definition looks like this:

  public function label() {
    if (isset($this->entityInfo['label callback']) && $this->entityInfo['label callback'] == 'entity_class_label') {
      return $this->defaultLabel();
    }
    return entity_label($this->entityType, $this);
  }

Since the label callback is no longer "entity_class_label" (because title changed it), this places a call to entity_label.

  $label = FALSE;
  $info = entity_get_info($entity_type);
  if (isset($info['label callback']) && function_exists($info['label callback'])) {
    $label = $info['label callback']($entity, $entity_type);
  }
  elseif (!empty($info['entity keys']['label']) && isset($entity->{$info['entity keys']['label']})) {
    $label = $entity->{$info['entity keys']['label']};
  }
  return $label;

And, as you can imagine, label callback is set to title_entity_label which starts the recursion and memory eating destruction.

It seems like this makes it difficult to override label callback for entities that use entity_label_class if you intend to use entity_label_class as a fallback (and possibly other cases as well).

I imagine the solution to be either altering the fallback to handle this case better or moving this issue over to the Entity module so they can have a more robust method of handling class-based labels without the risk of recursion where the label callback has been overridden (and yet is still used).

plach’s picture

@ottawadeveloper:

Thanks for the thorough report :)

I imagine the solution to be either altering the fallback to handle this case better or moving this issue over to the Entity module so they can have a more robust method of handling class-based labels without the risk of recursion where the label callback has been overridden (and yet is still used).

I'd say the long term solution should be the latter. We can introduce a stop-gap fix in Title to avoid the fallback when the label callback is 'entity_class_label', but this would defeat the goal of the original patch in this case. Actually the structure of the fallback key was designed to allow for a fallback chain where every module could deal with only a single override, that is the one it finds in the 'label callback' when its implementation of hook_entity_info_alter() is invoked. Perhaps the proper solution could be for Entity to adopt this "standard" and explicitly support the 'label fallback' key.

plach’s picture

Perhaps the proper solution could be for Entity to adopt this "standard" and explicitly support the 'label fallback' key.

Nope, probably this won't work, since it is supporting the use case when $entity->label() is not the label callback. It's not a matter of fallback. I guess Entity should just provide protection against recursion with a static variable or a class field.

plach’s picture

Title: Incompatibility with Bean module » Provide protection against label callback recursion
Project: Title » Entity API
Version: 7.x-1.0-alpha7 » 7.x-1.x-dev
Component: Code » Core integration
Status: Active » Needs review
FileSize
833 bytes

The attached patch should do the trick.

plach’s picture

thim’s picture

Hi Plach,

I can confirm that this issue resolved my error.

Here some more details so that other people can find the issue, in case of same problems.

After upgrading the Title module to Title 7.x-1.0-alpha7 and having Entity API 7.x-1.0 already in use, the following error occured.

[17-Apr-2013 11:46:25] PHP Fatal error: Maximum function nesting level of '200' reached, aborting! in /www/includes/common.inc on line 120
...
[17-Apr-2013 11:46:25] PHP 196. entity_class_label() /www/sites/all/modules/contrib/title/title.module:132
[17-Apr-2013 11:46:25] PHP 197. Entity->label() /www/sites/all/modules/contrib/entity/entity.module:1066
[17-Apr-2013 11:46:25] PHP 198. entity_label() /www/sites/all/modules/contrib/entity/includes/entity.inc:125
[17-Apr-2013 11:46:25] PHP 199. title_entity_label() /www/includes/common.inc:7890

The patch of Plach in comment #5 resolved it.
http://drupal.org/node/1947142#comment-7207826

FrancescoQ’s picture

Status: Needs review » Reviewed & tested by the community

Looks good and fixes the issue.

fago’s picture

Status: Reviewed & tested by the community » Needs work

Thanks, that looks like a good fix. But it might be a bit hard to follow, could we add some comments for explaining what we are doing here?

plach’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.26 KB

Added a comment explaining what's happening.

fago’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

Update based on my confirmation of which version we can revert to to fix the problem.