From fd1ddad98df45bc3c18be7980c1cbe68ce6b219c Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Sat, 26 Sep 2020 14:18:01 +0200 Subject: Add mutex on datastore I/O operations To make sure that there is no concurrent operation on the datastore file. Fixes #1132 --- application/api/ApiMiddleware.php | 2 ++ application/bookmark/BookmarkFileService.php | 9 ++++-- application/bookmark/BookmarkIO.php | 35 +++++++++++++++++------ application/bookmark/BookmarkServiceInterface.php | 11 ------- application/container/ContainerBuilder.php | 2 ++ 5 files changed, 38 insertions(+), 21 deletions(-) (limited to 'application') 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 @@ 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 = '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 @@ -15,15 +13,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 * 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() ); }; -- cgit v1.2.3