Race condition in locking framework

I am seeing this too. so using the lock through memcache is unworkable right now

moving the right direction.

But there is another error

PHP Fatal error: Call to undefined function lock_acquire() in modules/all/memcache/memcache.inc on line 96

The lines are

  // On cache misses, attempt to avoid stampedes when the
    // memcache_stampede_protection variable is enabled.
    if (!$cache) {
      if (variable_get('memcache_stampede_protection', FALSE) && !lock_acquire(\
"memcache_$cid:$this->bin", variable_get('memcache_stampede_semaphore', 15))) {
	

I was able to duplicate this by enabling page_cache_without_database and hitting a page that's not currently in the cache. The attached patch fixes for me, please test.

Actually, we need to load it properly through the bootstrap or our lock is going to get lost when we reinit during bootstrap. Fix committed:
http://drupalcode.org/project/memcache.git/commitdiff/1279c66

Sorry to re-open this, but a typo is introduced... $this->lockInit instead of $this->lockInit()

This patch fixes it.

Applies cleanly and fixes the issue.

Ouch, thanks!

Patch applied:
http://drupalcode.org/project/memcache.git/commitdiff/9582289

This effectively breaks all sites using stampede protection. A BIG do not upgrade or disable stampede protection is in order:

The reason is:

if ($this->lockInit() && ...

is always FALSE, because lockInit() does not return TRUE;

The consequence is that any expired cache item WILL ALWAYS BE RETURNED, until memcache is restarted, which is effectively a never expiring cache.

There is no cache_clear_all() or anything that will fix that condition - ever, because everything after the expired item is in else blocks and never executed.

diff --git a/memcache.inc b/memcache.inc
--- a/memcache.inc
+++ b/memcache.inc
@@ -463,6 +463,9 @@ class MemCacheDrupal implements DrupalCacheInterface {
 
   /**
    * Helper function to load locking framework if not already loaded.
+   *
+   * @return bool
+   *   Whether the locking system was initialized successfully.
    */
   public function lockInit() {
     // On a cache miss when page_cache_without_database is enabled, we can end
@@ -471,6 +474,8 @@ class MemCacheDrupal implements DrupalCacheInterface {
     if (!function_exists('lock_acquire')) {
       drupal_bootstrap(DRUPAL_BOOTSTRAP_VARIABLES, FALSE);
     }
+
+    return TRUE;
   }
 
   /**

is the proper fix. This makes it possible to keep this->lockInit() within it if () call, which is important to only load it on demand for the second scenario where it is called.

This implements #13.

Better PHP doc

RTBC - works locally for me

I guess this is fixed :)

Back to normal