#553952 by c960657: Apply locking framework to image generation.

2 files changed, 25 insertions, 23 deletions

diff --git a/modules/image/image.module b/modules/image/image.module
index b359254b5..3e43ed5f5 100644
--- a/modules/image/image.module
+++ b/modules/image/image.module
@@ -445,33 +445,34 @@ function image_style_generate() {
     exit();
   }
 
-  // Don't start generating the image if it is already in progress.
-  $cid = 'generate:' . $style_name . ':' . $path_md5;
-  if (cache_get($cid, 'cache_image')) {
-    // Tell client to retry again in 3 seconds. Currently no browsers are known
-    // to support Retry-After.
-    drupal_set_header('503 Service Unavailable');
-    drupal_set_header('Retry-After', 3);
-    print t('Image generation in progress, please try again shortly.');
-    exit();
+  // Don't start generating the image if the derivate already exists or if
+  // generation is in progress in another thread.
+  $lock_name = 'image_style_generate:' . $style_name . ':' . $path_md5;
+  if (!file_exists($destination)) {
+    $lock_acquired = lock_acquire($lock_name);
+    if (!$lock_acquired) {
+      // Tell client to retry again in 3 seconds. Currently no browsers are known
+      // to support Retry-After.
+      drupal_set_header('503 Service Unavailable');
+      drupal_set_header('Retry-After', 3);
+      print t('Image generation in progress, please try again shortly.');
+      exit();
+    }
   }
 
-  // If the image has already been generated then send it.
-  if ($image = image_load($destination)) {
-    file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Content-Length' => $image->info['file_size']));
-  }
+  // Try to generate the image, unless another thread just did it while we were
+  // acquiring the lock.
+  $success = file_exists($destination) || image_style_create_derivative($style, $path, $destination);
 
-  // Set a cache entry designating this image as being in-process.
-  cache_set($cid, $destination, 'cache_image');
+  if ($lock_acquired) {
+    lock_release($lock_name);
+  }
 
-  // Try to generate the image.
-  if (image_style_create_derivative($style, $path, $destination)) {
+  if ($success) {
     $image = image_load($destination);
-    cache_clear_all($cid, 'cache_image');
     file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Content-Length' => $image->info['file_size']));
   }
   else {
-    cache_clear_all($cid, 'cache_image');
     watchdog('image', 'Unable to generate the derived image located at %path.', $destination);
     drupal_set_header('500 Internal Server Error');
     print t('Error generating image.');
diff --git a/modules/image/image.test b/modules/image/image.test
index 94ce353d3..cbdcb1e08 100644
--- a/modules/image/image.test
+++ b/modules/image/image.test
@@ -87,13 +87,13 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase {
     $this->assertEqual($actual_generate_url, $expected_generate_url, t('Got the generate URL for a non-existent file.'));
 
     // Fetch the URL that generates the file while another process appears to
-    // be generating the same file (this is signaled using cache_image).
-    $cid = 'generate:' . $this->style_name . ':' . md5($this->image_filepath);
-    cache_set($cid, $generated_path, 'cache_image');
+    // be generating the same file (this is signaled using a lock).
+    $lock_name = 'image_style_generate:' . $this->style_name . ':' . md5($this->image_filepath);
+    $this->assertTrue(lock_acquire($lock_name), t('Lock was acquired.'));
     $this->drupalGet($expected_generate_url);
     $this->assertResponse(503, t('Service Unavailable response received.'));
     $this->assertTrue($this->drupalGetHeader('Retry-After'), t('Retry-After header received.'));
-    cache_clear_all($cid, 'cache_image');
+    lock_release($lock_name);
 
     // Fetch the URL that generates the file.
     $this->drupalGet($expected_generate_url);
@@ -102,6 +102,7 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase {
     $generated_image_info = image_get_info($generated_path);
     $this->assertEqual($this->drupalGetHeader('Content-Type'), $generated_image_info['mime_type'], t('Expected Content-Type was reported.'));
     $this->assertEqual($this->drupalGetHeader('Content-Length'), $generated_image_info['file_size'], t('Expected Content-Length was reported.'));
+    $this->assertTrue(lock_may_be_available($lock_name), t('Lock was released.'));
 
     // Check that the URL points directly to the generated file.
     $expected_generated_url = file_create_url($generated_path);