Protect from race conditions happening when the same FsBlob is open multiple times

src/filesystem/fsblobstore/DirBlob.cpp

@@ -28,8 +28,8 @@ namespace fsblobstore {
 
     //TODO Factor out a DirEntryList class
 
-DirBlob::DirBlob(unique_ref<Blob> blob, FsBlobStore *fsBlobStore) :
+DirBlob::DirBlob(unique_ref<Blob> blob, FsBlobStore *fsBlobStore, std::function<void()> onDestruct) :
-    FsBlob(std::move(blob)), _fsBlobStore(fsBlobStore), _entries(), _changed(false) {
+    FsBlob(std::move(blob), onDestruct), _fsBlobStore(fsBlobStore), _entries(), _changed(false) {
   ASSERT(magicNumber() == MagicNumbers::DIR, "Loaded blob is not a directory");
   _readEntriesFromBlob();
 }
@@ -43,9 +43,9 @@ void DirBlob::flush() {
   baseBlob().flush();
 }
 
-unique_ref<DirBlob> DirBlob::InitializeEmptyDir(unique_ref<Blob> blob, FsBlobStore *fsBlobStore) {
+unique_ref<DirBlob> DirBlob::InitializeEmptyDir(unique_ref<Blob> blob, FsBlobStore *fsBlobStore, std::function<void()> onDestruct) {
   InitializeBlobWithMagicNumber(blob.get(), MagicNumbers::DIR);
-  return make_unique_ref<DirBlob>(std::move(blob), fsBlobStore);
+  return make_unique_ref<DirBlob>(std::move(blob), fsBlobStore, onDestruct);
 }
 
 void DirBlob::_writeEntriesToBlob() {

src/filesystem/fsblobstore/DirBlob.h

@@ -43,9 +43,10 @@ namespace cryfs {
             };
 
             static cpputils::unique_ref<DirBlob> InitializeEmptyDir(cpputils::unique_ref<blobstore::Blob> blob,
-                                                                    FsBlobStore *fsBlobStore);
+                                                                    FsBlobStore *fsBlobStore,
+                                                                    std::function<void()> onDestruct);
 
-            DirBlob(cpputils::unique_ref<blobstore::Blob> blob, FsBlobStore *fsBlobStore);
+            DirBlob(cpputils::unique_ref<blobstore::Blob> blob, FsBlobStore *fsBlobStore, std::function<void()> onDestruct);
 
             virtual ~DirBlob();
 

src/filesystem/fsblobstore/FileBlob.cpp

@@ -11,13 +11,13 @@ using cpputils::make_unique_ref;
 namespace cryfs {
 namespace fsblobstore {
 
-FileBlob::FileBlob(unique_ref<Blob> blob)
+FileBlob::FileBlob(unique_ref<Blob> blob, std::function<void()> onDestruct)
-: FsBlob(std::move(blob)) {
+: FsBlob(std::move(blob), onDestruct) {
 }
 
-unique_ref<FileBlob> FileBlob::InitializeEmptyFile(unique_ref<Blob> blob) {
+unique_ref<FileBlob> FileBlob::InitializeEmptyFile(unique_ref<Blob> blob, std::function<void()> onDestruct) {
   InitializeBlobWithMagicNumber(blob.get(), MagicNumbers::FILE);
-  return make_unique_ref<FileBlob>(std::move(blob));
+  return make_unique_ref<FileBlob>(std::move(blob), onDestruct);
 }
 
 ssize_t FileBlob::read(void *target, uint64_t offset, uint64_t count) const {

src/filesystem/fsblobstore/FileBlob.h

@@ -9,9 +9,9 @@ namespace cryfs {
 
         class FileBlob: public FsBlob {
         public:
-            static cpputils::unique_ref<FileBlob> InitializeEmptyFile(cpputils::unique_ref<blobstore::Blob> blob);
+            static cpputils::unique_ref<FileBlob> InitializeEmptyFile(cpputils::unique_ref<blobstore::Blob> blob, std::function<void()> onDestruct);
 
-            FileBlob(cpputils::unique_ref<blobstore::Blob> blob);
+            FileBlob(cpputils::unique_ref<blobstore::Blob> blob, std::function<void()> onDestruct);
 
             ssize_t read(void *target, uint64_t offset, uint64_t count) const;
 

src/filesystem/fsblobstore/FsBlob.h

@@ -3,6 +3,7 @@
 
 #include <messmer/cpp-utils/pointer/unique_ref.h>
 #include <messmer/blobstore/interface/Blob.h>
+#include <functional>
 
 namespace cryfs {
     namespace fsblobstore {
@@ -14,7 +15,7 @@ namespace cryfs {
             blockstore::Key key() const;
 
         protected:
-            FsBlob(cpputils::unique_ref<blobstore::Blob> baseBlob);
+            FsBlob(cpputils::unique_ref<blobstore::Blob> baseBlob, std::function<void()> onDestruct);
 
             blobstore::Blob &baseBlob();
             const blobstore::Blob &baseBlob() const;
@@ -29,14 +30,17 @@ namespace cryfs {
             cpputils::unique_ref<blobstore::Blob> releaseBaseBlob();
 
             cpputils::unique_ref<blobstore::Blob> _baseBlob;
+            std::function<void()> _onDestruct;
 
             DISALLOW_COPY_AND_ASSIGN(FsBlob);
         };
 
-        inline FsBlob::FsBlob(cpputils::unique_ref<blobstore::Blob> baseBlob): _baseBlob(std::move(baseBlob)) {
+        inline FsBlob::FsBlob(cpputils::unique_ref<blobstore::Blob> baseBlob, std::function<void()> onDestruct)
+                : _baseBlob(std::move(baseBlob)), _onDestruct(onDestruct) {
         }
 
         inline FsBlob::~FsBlob() {
+            _onDestruct();
         }
 
         inline blockstore::Key FsBlob::key() const {

src/filesystem/fsblobstore/FsBlobStore.cpp

@@ -9,6 +9,7 @@ using cpputils::unique_ref;
 using cpputils::make_unique_ref;
 using blobstore::BlobStore;
 using boost::none;
+using std::function;
 
 namespace cryfs {
 namespace fsblobstore {
@@ -17,31 +18,42 @@ FsBlobStore::FsBlobStore(unique_ref<BlobStore> baseBlobStore): _baseBlobStore(st
 }
 
 unique_ref<FileBlob> FsBlobStore::createFileBlob() {
-    return FileBlob::InitializeEmptyFile(_baseBlobStore->create());
+    auto blob = _baseBlobStore->create();
+    auto key = blob->key();
+    _openBlobs.lock(key);
+    return FileBlob::InitializeEmptyFile(std::move(blob), freeLockFunction(key));
 }
 
 unique_ref<DirBlob> FsBlobStore::createDirBlob() {
+    auto blob = _baseBlobStore->create();
+    auto key = blob->key();
+    _openBlobs.lock(key);
     //TODO Passed in fsBlobStore should be ParallelAccessFsBlobStore later
-    return DirBlob::InitializeEmptyDir(_baseBlobStore->create(), this);
+    return DirBlob::InitializeEmptyDir(std::move(blob), this, freeLockFunction(key));
 }
 
 unique_ref<SymlinkBlob> FsBlobStore::createSymlinkBlob(const bf::path &target) {
-    return SymlinkBlob::InitializeSymlink(_baseBlobStore->create(), target);
+    auto blob = _baseBlobStore->create();
+    auto key = blob->key();
+    _openBlobs.lock(key);
+    return SymlinkBlob::InitializeSymlink(std::move(blob), target, freeLockFunction(key));
 }
 
 boost::optional<unique_ref<FsBlob>> FsBlobStore::load(const blockstore::Key &key) {
+    _openBlobs.lock(key);
+
     auto blob = _baseBlobStore->load(key);
     if (blob == none) {
         return none;
     }
     unsigned char magicNumber = FsBlob::magicNumber(**blob);
     if (magicNumber == MagicNumbers::FILE) {
-        return unique_ref<FsBlob>(make_unique_ref<FileBlob>(std::move(*blob)));
+        return unique_ref<FsBlob>(make_unique_ref<FileBlob>(std::move(*blob), freeLockFunction(key)));
     } else if (magicNumber == MagicNumbers::DIR) {
         //TODO Passed in fsBlobStore should be ParallelAccessFsBlobStore later
-        return unique_ref<FsBlob>(make_unique_ref<DirBlob>(std::move(*blob), this));
+        return unique_ref<FsBlob>(make_unique_ref<DirBlob>(std::move(*blob), this, freeLockFunction(key)));
     } else if (magicNumber == MagicNumbers::SYMLINK) {
-        return unique_ref<FsBlob>(make_unique_ref<SymlinkBlob>(std::move(*blob)));
+        return unique_ref<FsBlob>(make_unique_ref<SymlinkBlob>(std::move(*blob), freeLockFunction(key)));
     } else {
         ASSERT(false, "Unknown magic number");
     }
@@ -51,5 +63,11 @@ void FsBlobStore::remove(cpputils::unique_ref<FsBlob> blob) {
     _baseBlobStore->remove(blob->releaseBaseBlob());
 }
 
+function<void()> FsBlobStore::freeLockFunction(const blockstore::Key &key) {
+    return [this, key] {
+        _openBlobs.release(key);
+    };
+}
+
 }
 }

src/filesystem/fsblobstore/FsBlobStore.h

@@ -1,6 +1,7 @@
 #ifndef CRYFS_FSBLOBSTORE_FSBLOBSTORE_H
 #define CRYFS_FSBLOBSTORE_FSBLOBSTORE_H
 
+#include <messmer/cpp-utils/lock/LockPool.h>
 #include <messmer/cpp-utils/pointer/unique_ref.h>
 #include <messmer/blobstore/interface/BlobStore.h>
 #include "FileBlob.h"
@@ -9,6 +10,8 @@
 
 namespace cryfs {
     namespace fsblobstore {
+        //TODO Test classes in fsblobstore
+
         class FsBlobStore {
         public:
             FsBlobStore(cpputils::unique_ref<blobstore::BlobStore> baseBlobStore);
@@ -20,6 +23,10 @@ namespace cryfs {
             void remove(cpputils::unique_ref<FsBlob> blob);
 
         private:
+            std::function<void()> freeLockFunction(const blockstore::Key &key);
+
+            //Instead of locking open blobs, it would be faster to allow parallel access similar to parallelaccessstore.
+            cpputils::LockPool<blockstore::Key> _openBlobs;
             cpputils::unique_ref<blobstore::BlobStore> _baseBlobStore;
         };
     }

src/filesystem/fsblobstore/SymlinkBlob.cpp

@@ -14,17 +14,17 @@ namespace bf = boost::filesystem;
 namespace cryfs {
 namespace fsblobstore {
 
-SymlinkBlob::SymlinkBlob(unique_ref<Blob> blob)
+SymlinkBlob::SymlinkBlob(unique_ref<Blob> blob, std::function<void()> onDestruct)
-: FsBlob(std::move(blob)), _target(_readTargetFromBlob(baseBlob())) {
+: FsBlob(std::move(blob), onDestruct), _target(_readTargetFromBlob(baseBlob())) {
 }
 
-unique_ref<SymlinkBlob> SymlinkBlob::InitializeSymlink(unique_ref<Blob> blob, const bf::path &target) {
+unique_ref<SymlinkBlob> SymlinkBlob::InitializeSymlink(unique_ref<Blob> blob, const bf::path &target, std::function<void()> onDestruct) {
   string targetStr = target.native();
   blob->resize(1 + targetStr.size());
   unsigned char magicNumber = MagicNumbers::SYMLINK;
   blob->write(&magicNumber, 0, 1);
   blob->write(targetStr.c_str(), 1, targetStr.size());
-  return make_unique_ref<SymlinkBlob>(std::move(blob));
+  return make_unique_ref<SymlinkBlob>(std::move(blob), onDestruct);
 }
 
 void SymlinkBlob::_checkMagicNumber(const Blob &blob) {

src/filesystem/fsblobstore/SymlinkBlob.h

@@ -11,9 +11,10 @@ namespace cryfs {
         class SymlinkBlob: public FsBlob {
         public:
             static cpputils::unique_ref<SymlinkBlob> InitializeSymlink(cpputils::unique_ref<blobstore::Blob> blob,
-                                                                        const boost::filesystem::path &target);
+                                                                       const boost::filesystem::path &target,
+                                                                       std::function<void()> onDestruct);
 
-            SymlinkBlob(cpputils::unique_ref<blobstore::Blob> blob);
+            SymlinkBlob(cpputils::unique_ref<blobstore::Blob> blob, std::function<void()> onDestruct);
 
             const boost::filesystem::path &target() const;
 

src/main.cpp

@@ -27,6 +27,7 @@ using std::endl;
 using std::vector;
 
 //TODO Support files > 4GB
+//TODO cryfs process doesn't seem to react to "kill". Needs "kill -9". Why? Furthermore, calling "fusermount -u" unmounts the fs, but the cryfs process keeps running. Why?
 
 void showVersion() {
     cout << "CryFS Version " << version::VERSION_STRING << endl;