Skip to content

Commit

Permalink
CBL-6238: DataFile::documentKeys() is not thread-safe.
Browse files Browse the repository at this point in the history
Cherry-pick 02a5393 from CBL-6169 (#2132).

_documentKeys is initialized lazily without protection. We fix it by call_once.
  • Loading branch information
jianminzhao authored and callumbirks committed Sep 25, 2024
1 parent 5c78a6d commit 534e649
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 16 deletions.
28 changes: 15 additions & 13 deletions LiteCore/Storage/DataFile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ namespace litecore {
return ret;
}


#pragma mark - DATAFILE:


Expand Down Expand Up @@ -190,7 +190,7 @@ namespace litecore {
// getKeyStore will reopen them on demand
i.second->close();
}

_shared->addDataFile(this);
}

Expand Down Expand Up @@ -299,7 +299,7 @@ namespace litecore {
std::this_thread::sleep_for(100ms);
}
}

if (file)
file->close(true);
bool result = factory._deleteFile(FilePath(shared->path), options);
Expand Down Expand Up @@ -350,7 +350,7 @@ namespace litecore {
logDebug("close KVS '%s'", name.c_str());
auto i = _keyStores.find(name);
if (i != _keyStores.end()) {
// Never remove a KeyStore from _keyStores: there may be objects pointing to it
// Never remove a KeyStore from _keyStores: there may be objects pointing to it
i->second->close();
}
}
Expand All @@ -369,13 +369,15 @@ namespace litecore {


fleece::impl::SharedKeys* DataFile::documentKeys() const {
auto keys = _documentKeys.get();
if (!keys && _options.useDocumentKeys) {
auto mutableThis = const_cast<DataFile*>(this);
keys = new DocumentKeys(*mutableThis);
_documentKeys = keys;
}
return keys;
std::call_once(_documentKeysOnce, [this] {
auto keys = _documentKeys.get();
if ( !keys && _options.useDocumentKeys ) {
auto mutableThis = const_cast<DataFile*>(this);
keys = new DocumentKeys(*mutableThis);
_documentKeys = keys;
}
});
return _documentKeys.get();
}

#pragma mark - QUERIES:
Expand Down Expand Up @@ -403,7 +405,7 @@ namespace litecore {

#pragma mark - TRANSACTION:


void DataFile::beginTransactionScope(ExclusiveTransaction* t) {
Assert(!_inTransaction);
checkOpen();
Expand All @@ -430,7 +432,7 @@ namespace litecore {
ks.transactionWillEnd(committing);
});
}

void DataFile::endTransactionScope(ExclusiveTransaction* t) {
_shared->unsetTransaction(t);
_inTransaction = false;
Expand Down
7 changes: 4 additions & 3 deletions LiteCore/Storage/DataFile.hh
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ namespace litecore {
void forOtherDataFiles(function_ref<void(DataFile*)> fn);

//////// QUERIES:

/** Creates a database query object. */
virtual Retained<Query> compileQuery(slice expr,
QueryLanguage =QueryLanguage::kJSON,
Expand Down Expand Up @@ -203,7 +203,7 @@ namespace litecore {

/** Does a file exist at this path? */
virtual bool fileExists(const FilePath &path);

protected:
/** Deletes a non-open file. Returns false if it doesn't exist. */
virtual bool _deleteFile(const FilePath &path, const Options* =nullptr) =0;
Expand Down Expand Up @@ -274,7 +274,7 @@ namespace litecore {

static bool deleteDataFile(DataFile *file, const Options *options,
Shared *shared, Factory &factory);

KeyStore& addKeyStore(const std::string &name, KeyStore::Capabilities);
void closeAllQueries();
void beginTransactionScope(ExclusiveTransaction*);
Expand All @@ -297,6 +297,7 @@ namespace litecore {
std::mutex _queriesMutex; // Thread-safe access to _queries
bool _inTransaction {false}; // Am I in a Transaction?
std::atomic_bool _closeSignaled {false}; // Have I been asked to close?
mutable std::once_flag _documentKeysOnce{}; // Thread-safe init of documentKeys
};


Expand Down

0 comments on commit 534e649

Please sign in to comment.