MDL-32825 try to improve atomicity of cache file operations in themes and javascript

lib/csslib.php

@@ -71,10 +71,25 @@ function css_store_css(theme_config $theme, $csspath, array $cssfiles) {
         $css = $theme->post_process(css_minify_css($cssfiles));
     }
 
-    check_dir_exists(dirname($csspath));
-    $fp = fopen($csspath, 'w');
-    fwrite($fp, $css);
-    fclose($fp);
+    clearstatcache();
+    if (!file_exists(dirname($csspath))) {
+        @mkdir(dirname($csspath), $CFG->directorypermissions, true);
+    }
+
+    // Prevent serving of incomplete file from concurrent request,
+    // the rename() should be more atomic than fwrite().
+    ignore_user_abort(true);
+    if ($fp = fopen($csspath.'.tmp', 'xb')) {
+        fwrite($fp, $css);
+        fclose($fp);
+        rename($csspath.'.tmp', $csspath);
+        @chmod($csspath, $CFG->filepermissions);
+        @unlink($csspath.'.tmp'); // just in case anything fails
+    }
+    ignore_user_abort(false);
+    if (connection_aborted()) {
+        die;
+    }
 }
 
 /**

lib/javascript.php

@@ -88,19 +88,17 @@ if ($rev > -1) {
         js_send_cached($candidate, $etag);
 
     } else {
-        if (!file_exists(dirname($candidate))) {
-            @mkdir(dirname($candidate), $CFG->directorypermissions, true);
+        js_write_cache_file_content($candidate, js_minify($jsfiles));
+        // verify nothing failed in cache file creation
+        clearstatcache();
+        if (file_exists($candidate)) {
+            js_send_cached($candidate, $etag);
         }
-        $fp = fopen($candidate, 'w');
-        fwrite($fp, js_minify($jsfiles));
-        fclose($fp);
-        js_send_cached($candidate, $etag);
     }
-
-} else {
-    $content = '';
-    foreach ($jsfiles as $jsfile) {
-        $content .= file_get_contents($jsfile)."\n";
-    }
-    js_send_uncached($candidate, $etag);
 }
+
+$content = '';
+foreach ($jsfiles as $jsfile) {
+    $content .= file_get_contents($jsfile)."\n";
+}
+js_send_uncached($content, $etag);

lib/jslib.php

@@ -154,3 +154,40 @@ EOD;
     }
     return $js;
 }
+
+/**
+ * Create cache file for JS content
+ * @param string $file full file path to cache file
+ * @param string $content JS code
+ */
+function js_write_cache_file_content($file, $content) {
+    global $CFG;
+
+    clearstatcache();
+    if (!file_exists(dirname($file))) {
+        @mkdir(dirname($file), $CFG->directorypermissions, true);
+    }
+
+    // Prevent serving of incomplete file from concurrent request,
+    // the rename() should be more atomic than fwrite().
+    ignore_user_abort(true);
+    if ($fp = fopen($file.'.tmp', 'xb')) {
+        fwrite($fp, $content);
+        fclose($fp);
+        rename($file.'.tmp', $file);
+        @chmod($file, $CFG->filepermissions);
+        @unlink($file.'.tmp'); // just in case anything fails
+    }
+    ignore_user_abort(false);
+    if (connection_aborted()) {
+        die;
+    }
+}
+
+/**
+ * Sends a 404 message about CSS not being found.
+ */
+function js_send_css_not_found() {
+    header('HTTP/1.0 404 not found');
+    die('JS was not found, sorry.');
+}

theme/image.php

@@ -135,24 +135,36 @@ if (empty($imagefile) or !is_readable($imagefile)) {
     image_not_found();
 }
 
-
 if ($rev > -1) {
     $pathinfo = pathinfo($imagefile);
     $cacheimage = "$candidatelocation/$image.".$pathinfo['extension'];
-    if (!file_exists($cacheimage)) {
-        if (!file_exists(dirname($cacheimage))) {
-            // Sometimes there was a race condition in check_dir_exists(),
-            // so let the next copy() log errors instead.
-            @mkdir(dirname($cacheimage), $CFG->directorypermissions, true);
-        }
-        copy($imagefile, $cacheimage);
-    }
-    send_cached_image($cacheimage, $etag);
 
-} else {
-    send_uncached_image($imagefile);
+    clearstatcache();
+    if (!file_exists(dirname($cacheimage))) {
+        @mkdir(dirname($cacheimage), $CFG->directorypermissions, true);
+    }
+
+    // Prevent serving of incomplete file from concurrent request,
+    // the rename() should be more atomic than copy().
+    ignore_user_abort(true);
+    if (@copy($imagefile, $cacheimage.'.tmp')) {
+        rename($cacheimage.'.tmp', $cacheimage);
+        @chmod($cacheimage, $CFG->filepermissions);
+        @unlink($cacheimage.'.tmp'); // just in case anything fails
+    }
+    ignore_user_abort(false);
+    if (connection_aborted()) {
+        die;
+    }
+    // make sure nothing failed
+    clearstatcache();
+    if (file_exists($cacheimage)) {
+        send_cached_image($cacheimage, $etag);
+    }
 }
 
+send_uncached_image($imagefile);
+
 
 //=================================================================================
 //=== utility functions ==

theme/javascript.php

@@ -90,15 +90,12 @@ $rev = theme_get_revision();
 $etag = sha1("$themename/$rev/$type");
 
 if ($rev > -1) {
-    // note: cache reset might have purged our cache dir structure,
-    //       make sure we do not use stale file stat cache in the next check_dir_exists()
+    js_write_cache_file_content($candidate, js_minify($theme->javascript_files($type)));
+    // verify nothing failed in cache file creation
     clearstatcache();
-    check_dir_exists(dirname($candidate));
-    $fp = fopen($candidate, 'w');
-    fwrite($fp, js_minify($theme->javascript_files($type)));
-    fclose($fp);
-    js_send_cached($candidate, $etag);
-
-} else {
-    js_send_uncached($theme->javascript_content($type));
+    if (file_exists($candidate)) {
+        js_send_cached($candidate, $etag);
+    }
 }
+
+js_send_uncached($theme->javascript_content($type));

theme/styles.php

@@ -118,4 +118,11 @@ if ($type === 'editor') {
     $cssfile = "$CFG->cachedir/theme/$themename/css/all.css";
     css_store_css($theme, $cssfile, $allfiles);
 }
+
+// verify nothing failed in cache file creation
+clearstatcache();
+if (!file_exists($candidatesheet)) {
+    css_send_css_not_found();
+}
+
 css_send_cached_css($candidatesheet, $etag);