Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove recursive locking from variable caching #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Remove recursive locking from variable caching #52

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0d339d
Remove recursive locking from variable cache
mike-potter Aug 2, 2012
b29a82d
Fix new variable cache with proper use of static variable
mike-potter Aug 6, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 108 additions & 46 deletions includes/bootstrap.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,40 +560,96 @@ function drupal_get_filename($type, $name, $filename = NULL) {
* with variable_set() as well as those explicitly specified in the configuration
* file.
*/
function variable_init($conf = array(), $regenerate = FALSE, $recursion_depth = 0) {
function variable_init($conf = array()) {
// using locks will install a shutdown function locks_release_all,
// so we install our variables_reset_cache shutdown function first
// to prevent locks from being released before it runs.
variable_cache_rebuild_shutdown();

// NOTE: caching the variables improves performance by 20% when serving cached pages.
if (!$regenerate && $cached = cache_get('variables', 'cache')) {
if ($cached = cache_get('variables', 'cache')) {
$variables = $cached->data;
}
else {
if (defined('MAINTENANCE_MODE') || lock_acquire('variable_cache_regenerate')) {
$result = db_query('SELECT * FROM {variable}');
while ($variable = db_fetch_object($result)) {
$variables[$variable->name] = unserialize($variable->value);
// grab the variables table and set the cache
$variables = variable_reset_cache(TRUE);
}

foreach ($conf as $name => $value) {
$variables[$name] = $value;
}

return $variables;
}

/**
* Local helper function Get/Set a persistent flag to indicate that
* a variable_set or variable_del has been done
*/
function _variable_write($new_value = NULL) {
static $variable_write = FALSE;
if (!is_null($new_value)) {
$variable_write = $new_value;
}
return $variable_write;
}

/**
* Reset the variable cache safely.
*
* @param bool $force
* Flag to force cache to be rebuilt and variables returned no matter what
*/
function variable_reset_cache($force = FALSE) {
// $variable_write indicates that a variable_set or variable_del has taken place
static $variable_write = FALSE;

$variables = array();
// check to see if a variable_set or variable_del has been done
$variable_write = _variable_write();
if ($force || $variable_write) {
$error_msg = '';
$lock_name = __FUNCTION__;
// try to acquire a lock, just in case we can
$lock_acquired = lock_acquire($lock_name, 1);
try { // trap exceptions while we have the lock
// read current variables from DB
$result = db_query('SELECT name, value FROM {variable}');
if ($result) {
while ($variable = db_fetch_object($result)) {
$variables[$variable->name] = unserialize($variable->value);
}
}
cache_set('variables', $variables);
if (!defined('MAINTENANCE_MODE')) {
lock_release('variable_cache_regenerate');
else {
$error_msg = t('Failed to retrieve variables table.');
}
}
else {
// Wait for another request that is already doing this work.
lock_wait('variable_cache_regenerate');

// Run the function again. Try a limited number of times to avoid
// infinite recursion if the database connection is invalid for
// some reason, e.g., mysqld restart, loss of network, etc.
$recursion_depth++;
if ($recursion_depth < 50) {
return variable_init($conf, $regenerate, $recursion_depth);
catch (Exception $e) {
$error_msg = 'Exception: ' . $e->getMessage();
}
if (!empty($error_msg)) {
// make sure lock is released and cache is cleared if we get an exception
// or error reading from the database
if ($lock_acquired) {
lock_release($lock_name);
}

$variables = array();
if ($variable_write) {
cache_clear_all('variables', 'cache');
}
watchdog('variable_reset_cache', 'Error while locked: %msg', array('%msg' => $error_msg));
}
}

foreach ($conf as $name => $value) {
$variables[$name] = $value;
elseif ($lock_acquired) {
// if we acquired the lock, then update the drupal cache and release lock
cache_set('variables', $variables);
lock_release($lock_name);
}
elseif ($variable_write) {
// When we've made a change via variable_set() or variable_del() and failed
// to rebuild the cache, we need to clear it to ensure that subsequent
// requests pick up the changes.
cache_clear_all('variables', 'cache');
}
_variable_write(FALSE); // don't need to write cache until next variable_set
}

return $variables;
Expand Down Expand Up @@ -653,8 +709,11 @@ function variable_set($name, $value) {
if (is_string($db_prefix) && strpos($db_prefix, 'simpletest') === 0) {
cache_clear_all('variables', 'cache');
}

variable_cache_rebuild();
else {
// indicate to variable_reset_cache that a variable_set has occured
_variable_write(TRUE);
variable_cache_rebuild_shutdown(); // ensure shutdown function is registered
}
}

/**
Expand Down Expand Up @@ -682,17 +741,20 @@ function variable_del($name) {
if (is_string($db_prefix) && strpos($db_prefix, 'simpletest') === 0) {
cache_clear_all('variables', 'cache');
}

variable_cache_rebuild();
else {
// indicate to variable_reset_cache that a variable_del has occured
_variable_write(TRUE);
variable_cache_rebuild_shutdown(); // ensure shutdown function is registered
}
}

/**
* Schedules a rebuild of the variable cache on shutdown.
*/
function variable_cache_rebuild() {
function variable_cache_rebuild_shutdown() {
static $shutdown_registered = FALSE;
if (!$shutdown_registered) {
register_shutdown_function('variable_init', array(), TRUE);
register_shutdown_function('variable_reset_cache');
$shutdown_registered = TRUE;
}
}
Expand Down Expand Up @@ -805,7 +867,7 @@ function drupal_set_header($name = NULL, $value = NULL, $append = FALSE) {
if (!isset($name)) {
return $headers;
}

// Support the Drupal 6 header API
if (!isset($value)) {
if (strpos($name, ':') !== FALSE) {
Expand Down Expand Up @@ -1423,7 +1485,7 @@ function drupal_bootstrap($phase = NULL) {
_drupal_bootstrap($current_phase);
}
}

return $phase_index;
}

Expand Down Expand Up @@ -1483,7 +1545,7 @@ function _drupal_bootstrap($phase) {
// those using APC or memcached.
require_once variable_get('lock_inc', './includes/lock.inc');
lock_init();

// Detect if an installation is present.
detect_installation_or_run_installer();

Expand Down Expand Up @@ -1534,11 +1596,11 @@ function _drupal_bootstrap($phase) {
// We are done.
exit;
}

if (!$cache && drupal_page_is_cacheable() && $cache_mode != CACHE_EXTERNAL) {
header('X-Drupal-Cache: MISS');
}

// If using an external cache and the page is cacheable, set headers.
if ($cache_mode == CACHE_EXTERNAL && drupal_page_is_cacheable()) {
drupal_page_cache_header_external();
Expand Down Expand Up @@ -1679,17 +1741,17 @@ function ip_address() {

if (!isset($ip_address)) {
$ip_address = $_SERVER['REMOTE_ADDR'];

// Only use parts of the X-Forwarded-For (XFF) header that have followed a trusted route.
// Specifically, identify the leftmost IP address in the XFF header that is not one of ours.
// An XFF header is: X-Forwarded-For: client1, proxy1, proxy2
if (isset($_SERVER['HTTP_' . variable_get('x_forwarded_for_header', 'X_FORWARDED_FOR')]) && variable_get('reverse_proxy', 0)) {
// Load trusted reverse proxy server IPs.
$reverse_proxy_addresses = variable_get('reverse_proxy_addresses', array());

// Turn XFF header into an array.
$forwarded = explode(',', $_SERVER['HTTP_' . variable_get('x_forwarded_for_header', 'X_FORWARDED_FOR')]);

// Trim the forwarded IPs; they may have been delimited by commas and spaces.
$forwarded = array_map('trim', $forwarded);

Expand All @@ -1698,7 +1760,7 @@ function ip_address() {

// Eliminate all trusted IPs.
$untrusted = array_diff($forwarded, $reverse_proxy_addresses);

// The right-most IP is the most specific we can trust.
$ip_address = array_pop($untrusted);
}
Expand All @@ -1712,9 +1774,9 @@ function ip_address() {
*/
function drupal_session_initialize() {
global $user;

session_set_save_handler('sess_open', 'sess_close', 'sess_read', 'sess_write', 'sess_destroy_sid', 'sess_gc');

if (isset($_COOKIE[session_name()])) {
// If a session cookie exists, initialize the session. Otherwise the
// session is only started on demand in drupal_session_commit(), making
Expand Down Expand Up @@ -1776,7 +1838,7 @@ function drupal_session_commit() {

/**
* Return whether a session has been started.
*/
*/
function drupal_session_started($set = NULL) {
static $session_started = FALSE;
if (isset($set)) {
Expand Down Expand Up @@ -1841,7 +1903,7 @@ function drupal_save_session($status = NULL) {
}
return $save_session;
}

/**
* Returns the current bootstrap phase for this Drupal process.
*
Expand Down Expand Up @@ -1886,7 +1948,7 @@ function drupal_generate_test_ua($prefix) {
// check the HMAC before the database is initialized. filectime()
// and fileinode() are not easily determined from remote.
// $filepath = DRUPAL_ROOT . '/includes/bootstrap.inc';
$filepath = './includes/bootstrap.inc';
$filepath = './includes/bootstrap.inc';
// $key = sha1(serialize($databases) . filectime($filepath) . fileinode($filepath), TRUE);
$key = sha1(serialize($db_url) . filectime($filepath) . fileinode($filepath), TRUE);
}
Expand Down Expand Up @@ -1914,7 +1976,7 @@ function drupal_is_cli() {
*/
function drupal_session_destroy() {
session_destroy();

// Workaround PHP 5.2 fatal error "Failed to initialize storage module".
// @see http://bugs.php.net/bug.php?id=32330
session_set_save_handler('sess_open', 'sess_close', 'sess_read', 'sess_write', 'sess_destroy_sid', 'sess_gc');
Expand Down