Add mutex on datastore I/O operations

To make sure that there is no concurrent operation on the datastore file.

Fixes #1132

5 files changed, 38 insertions, 21 deletions

diff --git a/application/api/ApiMiddleware.php b/application/api/ApiMiddleware.php
index f5b53b01..adc8b266 100644
--- a/application/api/ApiMiddleware.php
+++ b/application/api/ApiMiddleware.php
@@ -1,6 +1,7 @@
 <?php
 namespace Shaarli\Api;
 
+use malkusch\lock\mutex\FlockMutex;
 use Shaarli\Api\Exceptions\ApiAuthorizationException;
 use Shaarli\Api\Exceptions\ApiException;
 use Shaarli\Bookmark\BookmarkFileService;
@@ -143,6 +144,7 @@ class ApiMiddleware
         $linkDb = new BookmarkFileService(
             $conf,
             $this->container->get('history'),
+            new FlockMutex(fopen(SHAARLI_MUTEX_FILE, 'r'), 2),
             true
         );
         $this->container['db'] = $linkDb;
diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php
index c9ec2609..1ba00712 100644
--- a/application/bookmark/BookmarkFileService.php
+++ b/application/bookmark/BookmarkFileService.php
@@ -5,6 +5,7 @@ namespace Shaarli\Bookmark;
 
 
 use Exception;
+use malkusch\lock\mutex\Mutex;
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
 use Shaarli\Bookmark\Exception\DatastoreNotInitializedException;
 use Shaarli\Bookmark\Exception\EmptyDataStoreException;
@@ -47,15 +48,19 @@ class BookmarkFileService implements BookmarkServiceInterface
     /** @var bool true for logged in users. Default value to retrieve private bookmarks. */
     protected $isLoggedIn;
 
+    /** @var Mutex */
+    protected $mutex;
+
     /**
      * @inheritDoc
      */
-    public function __construct(ConfigManager $conf, History $history, $isLoggedIn)
+    public function __construct(ConfigManager $conf, History $history, Mutex $mutex, $isLoggedIn)
     {
         $this->conf = $conf;
         $this->history = $history;
+        $this->mutex = $mutex;
         $this->pageCacheManager = new PageCacheManager($this->conf->get('resource.page_cache'), $isLoggedIn);
-        $this->bookmarksIO = new BookmarkIO($this->conf);
+        $this->bookmarksIO = new BookmarkIO($this->conf, $this->mutex);
         $this->isLoggedIn = $isLoggedIn;
 
         if (!$this->isLoggedIn && $this->conf->get('privacy.hide_public_links', false)) {
diff --git a/application/bookmark/BookmarkIO.php b/application/bookmark/BookmarkIO.php
index 6bf7f365..099653b0 100644
--- a/application/bookmark/BookmarkIO.php
+++ b/application/bookmark/BookmarkIO.php
@@ -2,6 +2,8 @@
 
 namespace Shaarli\Bookmark;
 
+use malkusch\lock\mutex\Mutex;
+use malkusch\lock\mutex\NoMutex;
 use Shaarli\Bookmark\Exception\DatastoreNotInitializedException;
 use Shaarli\Bookmark\Exception\EmptyDataStoreException;
 use Shaarli\Bookmark\Exception\NotWritableDataStoreException;
@@ -27,11 +29,14 @@ class BookmarkIO
      */
     protected $conf;
 
+
+    /** @var Mutex */
+    protected $mutex;
+
     /**
      * string Datastore PHP prefix
      */
     protected static $phpPrefix = '<?php /* ';
-
     /**
      * string Datastore PHP suffix
      */
@@ -42,10 +47,15 @@ class BookmarkIO
      *
      * @param ConfigManager $conf instance
      */
-    public function __construct($conf)
+    public function __construct(ConfigManager $conf, Mutex $mutex = null)
     {
+        if ($mutex === null) {
+            // This should only happen with legacy classes
+            $mutex = new NoMutex();
+        }
         $this->conf = $conf;
         $this->datastore = $conf->get('resource.datastore');
+        $this->mutex = $mutex;
     }
 
     /**
@@ -67,11 +77,16 @@ class BookmarkIO
             throw new NotWritableDataStoreException($this->datastore);
         }
 
+        $content = null;
+        $this->mutex->synchronized(function () use (&$content) {
+            $content = file_get_contents($this->datastore);
+        });
+
         // Note that gzinflate is faster than gzuncompress.
         // See: http://www.php.net/manual/en/function.gzdeflate.php#96439
         $links = unserialize(gzinflate(base64_decode(
-            substr(file_get_contents($this->datastore),
-                strlen(self::$phpPrefix), -strlen(self::$phpSuffix)))));
+            substr($content, strlen(self::$phpPrefix), -strlen(self::$phpSuffix))
+        )));
 
         if (empty($links)) {
             if (filesize($this->datastore) > 100) {
@@ -100,9 +115,13 @@ class BookmarkIO
             throw new NotWritableDataStoreException(dirname($this->datastore));
         }
 
-        file_put_contents(
-            $this->datastore,
-            self::$phpPrefix.base64_encode(gzdeflate(serialize($links))).self::$phpSuffix
-        );
+        $data = self::$phpPrefix.base64_encode(gzdeflate(serialize($links))).self::$phpSuffix;
+
+        $this->mutex->synchronized(function () use ($data) {
+            file_put_contents(
+                $this->datastore,
+                $data
+            );
+        });
     }
 }
diff --git a/application/bookmark/BookmarkServiceInterface.php b/application/bookmark/BookmarkServiceInterface.php
index b9b483eb..638cfa5f 100644
--- a/application/bookmark/BookmarkServiceInterface.php
+++ b/application/bookmark/BookmarkServiceInterface.php
@@ -5,8 +5,6 @@ namespace Shaarli\Bookmark;
 
 use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
 use Shaarli\Bookmark\Exception\NotWritableDataStoreException;
-use Shaarli\Config\ConfigManager;
-use Shaarli\History;
 
 /**
  * Class BookmarksService
@@ -16,15 +14,6 @@ use Shaarli\History;
 interface BookmarkServiceInterface
 {
     /**
-     * BookmarksService constructor.
-     *
-     * @param ConfigManager $conf       instance
-     * @param History       $history    instance
-     * @param bool          $isLoggedIn true if the current user is logged in
-     */
-    public function __construct(ConfigManager $conf, History $history, $isLoggedIn);
-
-    /**
      * Find a bookmark by hash
      *
      * @param string $hash
diff --git a/application/container/ContainerBuilder.php b/application/container/ContainerBuilder.php
index 55bb51b5..c21d58dd 100644
--- a/application/container/ContainerBuilder.php
+++ b/application/container/ContainerBuilder.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Shaarli\Container;
 
+use malkusch\lock\mutex\FlockMutex;
 use Shaarli\Bookmark\BookmarkFileService;
 use Shaarli\Bookmark\BookmarkServiceInterface;
 use Shaarli\Config\ConfigManager;
@@ -84,6 +85,7 @@ class ContainerBuilder
             return new BookmarkFileService(
                 $container->conf,
                 $container->history,
+                new FlockMutex(fopen(SHAARLI_MUTEX_FILE, 'r'), 2),
                 $container->loginManager->isLoggedIn()
             );
         };