Improved File::save() not to use file lock, but a temporary file, fixed Obtaining write lock failed on file...

CHANGELOG.md

@@ -5,11 +5,13 @@
     * Grav 1.6: Renamed `$grav['users']` service to `$grav['accounts']`
 1. [](#improved)
     * Renamed `Grav\Framework\File\Formatter\FormatterInterface` to `Grav\Framework\File\Interfaces\FileFormatterInterface`
+    * Improved `File::save()` not to use file lock, but a temporary file
 1. [](#bugfix)
     * Grav 1.6: Fixed `FlexUser` loosing ACL information
     * Grav 1.6: Fixed `FlexUser::find()` breaking when nothing is found
     * Grav 1.6: Fixed `FlexObject::update()` removing fields on save
     * Fixed `mkdir(...)` race condition
+    * Fixed `Obtaining write lock failed on file...`
 
 # v1.6.0-rc.3
 ## 02/18/2019

system/src/Grav/Framework/File/AbstractFile.php

@@ -199,8 +199,13 @@ class AbstractFile implements FileInterface
                 throw new \RuntimeException("Opening file for writing failed on error {$error['message']}");
             }
         }
+
         $lock = $block ? LOCK_EX : LOCK_EX | LOCK_NB;
-        return $this->locked = $this->handle ? flock($this->handle, $lock) : false;
+
+        // Some filesystems do not support file locks, only fail if another process holds the lock.
+        $this->locked = flock($this->handle, $lock, $wouldblock) || !$wouldblock;
+
+        return $this->locked;
     }
 
     /**
@@ -213,10 +218,12 @@ class AbstractFile implements FileInterface
         if (!$this->handle) {
             return false;
         }
+
         if ($this->locked) {
             flock($this->handle, LOCK_UN);
             $this->locked = false;
         }
+
         fclose($this->handle);
         $this->handle = null;
 
@@ -275,27 +282,24 @@ class AbstractFile implements FileInterface
      */
     public function save($data): void
     {
-        $lock = false;
-        if (!$this->locked) {
-            // Obtain blocking lock or fail.
-            if (!$this->lock()) {
-                throw new \RuntimeException('Obtaining write lock failed on file: ' . $this->filepath);
-            }
-            $lock = true;
+        $filepath = $this->filepath;
+        $dir = $this->getPath();
+
+        // Create file with a temporary name and rename it to make the save action atomic.
+        $tmp = tempnam($dir, basename($filepath));
+        if ($tmp && @file_put_contents($tmp, $data) && @rename($tmp, $filepath)) {
+            @chmod($filepath, 0666 & ~umask());
+        } elseif ($tmp) {
+            @unlink($tmp);
+            $tmp = false;
         }
 
-        // As we are using non-truncating locking, make sure that the file is empty before writing.
-        if (@ftruncate($this->handle, 0) === false || @fwrite($this->handle, $data) === false) {
-            $this->unlock();
-            throw new \RuntimeException('Saving file failed: ' . $this->filepath);
-        }
-
-        if ($lock) {
-            $this->unlock();
+        if ($tmp === false) {
+            throw new \RuntimeException('Failed to save file ' . $filepath);
         }
 
         // Touch the directory as well, thus marking it modified.
-        @touch($this->getPath());
+        @touch($dir);
     }
 
     /**

system/src/Grav/Framework/Flex/FlexIndex.php

@@ -558,6 +558,7 @@ class FlexIndex extends ObjectIndex implements FlexCollectionInterface, FlexInde
         static::onChanges($index, $added, $updated, $removed);
 
         $indexFile->save(['count' => \count($index), 'index' => $index]);
+        $indexFile->unlock();
 
         return $index;
     }