variable_realm() function returns value by reference and uses static variable $variable_realm that in turn is a reference to drupal_static storage.

Due to PHP behavior http://www.php.net/manual/en/language.variables.scope.php#language.varia... you cannot have a static reference variable:

A similar behaviour applies to the static statement. References are not stored statically:

Also, take a look at Drupal core examples.

As a result switch variables realms is not working properly $variable_realm['default']['config'] is being overridden by current $GLOBALS['conf'] on each call.

CommentFileSizeAuthor
#3 variable-wrong-drupal-static-utilization-1371906-3.patch1.15 KBtaran2l
#1 wrong-drupal-static-utilization-1371906-1.patch875 bytestaran2l

Comments

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
StatusFileSize
new wrong-drupal-static-utilization-1371906-1.patch875 bytes

Patch attached.

Please review.

jose reyero’s picture

jose reyero commented
Status: Active » Postponed (maintainer needs more info)

That drupal_static() usage should be ok and it is documented, see 'advanced drupal_static() pattern', http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

However you may be right about the static reference to a global variable, though I don't think your patch will fix that issue as it just changes how the variable is initialized but not the assignmet of the global variable.

We'd need a more specific (may be code) example of this issue to see when this is failing.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
StatusFileSize
new variable-wrong-drupal-static-utilization-1371906-3.patch1.15 KB

Advanced drupal_static() pattern requires use of an array() wrapper (and this moment is described in the docs).

My previous patch actually fixes the problem, but switches drupal_static() storage to use simple pattern.

Attached new version with correct use of advanced drupal_static() pattern. Please review.

Couple more details. Problem is in these lines: 

  static $variable_realm;
  // this condition is always TRUE, and that's why global default realm is being overridden over and over again.
  if (!isset($variable_realm)) {
    $variable_realm = &drupal_static(__FUNCTION__);
    $variable_realm['global']['default'] = $GLOBALS['conf'];
  }

The new patch uses work around from http://api.drupal.org/api/drupal/includes--bootstrap.inc/function/drupal...

jose reyero’s picture

jose reyero commented
Status: Postponed (maintainer needs more info) » Fixed

Ok, got it. Committed, thanks.

Btw, fixed variable_store() the same way.

Status: Fixed » Closed (fixed)

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