diff --git a/arangod/ClusterEngine/ClusterTransactionCollection.cpp b/arangod/ClusterEngine/ClusterTransactionCollection.cpp index 0e5a6c67a9..7b60686691 100644 --- a/arangod/ClusterEngine/ClusterTransactionCollection.cpp +++ b/arangod/ClusterEngine/ClusterTransactionCollection.cpp @@ -168,9 +168,8 @@ int ClusterTransactionCollection::use(int nestingLevel) { } try { - _sharedCollection = ci->getCollection(_transaction->vocbase().name(), std::to_string(_cid)); - if (_sharedCollection) { - _collection = _sharedCollection.get(); + _collection = ci->getCollection(_transaction->vocbase().name(), std::to_string(_cid)); + if (_collection) { if (!_transaction->hasHint(transaction::Hints::Hint::LOCK_NEVER) && !_transaction->hasHint(transaction::Hints::Hint::NO_USAGE_LOCK)) { // use and usage-lock @@ -224,7 +223,7 @@ void ClusterTransactionCollection::release() { LOG_TRX(_transaction, 0) << "unusing collection " << _cid; if (_usageLocked) { - _transaction->vocbase().releaseCollection(_collection); + _transaction->vocbase().releaseCollection(_collection.get()); _usageLocked = false; } _collection = nullptr; @@ -257,9 +256,7 @@ int ClusterTransactionCollection::doLock(AccessMode::Type type, TRI_ASSERT(!isLocked()); - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); - + TRI_ASSERT(_collection); LOG_TRX(_transaction, nestingLevel) << "write-locking collection " << _cid; _lockType = type; @@ -310,8 +307,7 @@ int ClusterTransactionCollection::doUnlock(AccessMode::Type type, return TRI_ERROR_INTERNAL; } - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); + TRI_ASSERT(_collection); _lockType = AccessMode::Type::NONE; diff --git a/arangod/ClusterEngine/ClusterTransactionCollection.h b/arangod/ClusterEngine/ClusterTransactionCollection.h index 855b32189e..20b50615c6 100644 --- a/arangod/ClusterEngine/ClusterTransactionCollection.h +++ b/arangod/ClusterEngine/ClusterTransactionCollection.h @@ -89,8 +89,6 @@ class ClusterTransactionCollection final : public TransactionCollection { AccessMode::Type _lockType; // collection lock type, used for exclusive locks int _nestingLevel; // the transaction level that added this collection bool _usageLocked; // is this already locked - /// @brief shared ptr to the collection so we can safely use _collection - std::shared_ptr _sharedCollection; }; } diff --git a/arangod/MMFiles/MMFilesTransactionCollection.cpp b/arangod/MMFiles/MMFilesTransactionCollection.cpp index 3473204e3a..6c07409a9e 100644 --- a/arangod/MMFiles/MMFilesTransactionCollection.cpp +++ b/arangod/MMFiles/MMFilesTransactionCollection.cpp @@ -218,7 +218,7 @@ int MMFilesTransactionCollection::use(int nestingLevel) { } } else { // use without usage-lock (lock already set externally) - _collection = _transaction->vocbase().lookupCollection(_cid).get(); + _collection = _transaction->vocbase().lookupCollection(_cid); if (_collection == nullptr) { return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND; @@ -300,7 +300,7 @@ void MMFilesTransactionCollection::release() { // unuse collection, remove usage-lock LOG_TRX(_transaction, 0) << "unusing collection " << _cid; - _transaction->vocbase().releaseCollection(_collection); + _transaction->vocbase().releaseCollection(_collection.get()); _collection = nullptr; } } @@ -323,11 +323,9 @@ int MMFilesTransactionCollection::doLock(AccessMode::Type type, int nestingLevel } TRI_ASSERT(!isLocked()); - - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); - - auto physical = static_cast(collection->getPhysical()); + TRI_ASSERT(_collection); + + auto physical = static_cast(_collection->getPhysical()); TRI_ASSERT(physical != nullptr); double timeout = _transaction->timeout(); @@ -400,10 +398,8 @@ int MMFilesTransactionCollection::doUnlock(AccessMode::Type type, int nestingLev bool const useDeadlockDetector = (!_transaction->hasHint(transaction::Hints::Hint::SINGLE_OPERATION) && !_transaction->hasHint(transaction::Hints::Hint::NO_DLD)); - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); - - auto physical = static_cast(collection->getPhysical()); + TRI_ASSERT(_collection); + auto physical = static_cast(_collection->getPhysical()); TRI_ASSERT(physical != nullptr); if (!AccessMode::isWriteOrExclusive(_lockType)) { diff --git a/arangod/MMFiles/MMFilesWalRecoverState.cpp b/arangod/MMFiles/MMFilesWalRecoverState.cpp index 5f3f9afe63..7379dabc4a 100644 --- a/arangod/MMFiles/MMFilesWalRecoverState.cpp +++ b/arangod/MMFiles/MMFilesWalRecoverState.cpp @@ -109,8 +109,8 @@ void MMFilesWalRecoverState::releaseResources() { // release all collections for (auto it = openedCollections.begin(); it != openedCollections.end(); ++it) { - arangodb::LogicalCollection* collection = it->second; - collection->vocbase().releaseCollection(collection); + std::shared_ptr& collection = it->second; + collection->vocbase().releaseCollection(collection.get()); } openedCollections.clear(); @@ -158,14 +158,14 @@ TRI_vocbase_t* MMFilesWalRecoverState::releaseDatabase( auto it2 = openedCollections.begin(); while (it2 != openedCollections.end()) { - arangodb::LogicalCollection* collection = it2->second; + std::shared_ptr& collection = it2->second; TRI_ASSERT(collection != nullptr); if (collection->vocbase().id() == databaseId) { // correct database, now release the collection TRI_ASSERT(vocbase == &(collection->vocbase())); - vocbase->releaseCollection(collection); + vocbase->releaseCollection(collection.get()); // get new iterator position it2 = openedCollections.erase(it2); } else { @@ -181,7 +181,7 @@ TRI_vocbase_t* MMFilesWalRecoverState::releaseDatabase( } /// @brief release a collection (so it can be dropped) -arangodb::LogicalCollection* MMFilesWalRecoverState::releaseCollection( +std::shared_ptr MMFilesWalRecoverState::releaseCollection( TRI_voc_cid_t collectionId) { auto it = openedCollections.find(collectionId); @@ -189,10 +189,9 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::releaseCollection( return nullptr; } - arangodb::LogicalCollection* collection = it->second; - + std::shared_ptr& collection = it->second; TRI_ASSERT(collection != nullptr); - collection->vocbase().releaseCollection(collection); + collection->vocbase().releaseCollection(collection.get()); openedCollections.erase(collectionId); return collection; @@ -205,12 +204,12 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::useCollection( if (it != openedCollections.end()) { res = TRI_ERROR_NO_ERROR; - return (*it).second; + return (*it).second.get(); } TRI_set_errno(TRI_ERROR_NO_ERROR); TRI_vocbase_col_status_e status; // ignored here - arangodb::LogicalCollection* collection = + std::shared_ptr collection = vocbase->useCollection(collectionId, status); if (collection == nullptr) { @@ -232,7 +231,7 @@ arangodb::LogicalCollection* MMFilesWalRecoverState::useCollection( openedCollections.emplace(collectionId, collection); res = TRI_ERROR_NO_ERROR; - return collection; + return collection.get(); } /// @brief looks up a collection @@ -672,11 +671,9 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, return true; } - arangodb::LogicalCollection* collection = - state->releaseCollection(collectionId); - - if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId).get(); + auto collection = state->releaseCollection(collectionId); + if (!collection) { + collection = vocbase->lookupCollection(collectionId); } if (collection == nullptr) { @@ -995,11 +992,9 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, return true; } - arangodb::LogicalCollection* collection = - state->releaseCollection(collectionId); - - if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId).get(); + auto collection = state->releaseCollection(collectionId); + if (!collection) { + collection = vocbase->lookupCollection(collectionId); } if (collection != nullptr) { @@ -1017,7 +1012,7 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, if (nameSlice.isString()) { name = nameSlice.copyString(); - collection = vocbase->lookupCollection(name).get(); + collection = vocbase->lookupCollection(name); if (collection != nullptr) { auto otherCid = collection->id(); @@ -1058,10 +1053,10 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, // restore the old behavior afterwards TRI_DEFER(state->databaseFeature->forceSyncProperties(oldSync)); - collection = vocbase->createCollection(b2.slice()).get(); + collection = vocbase->createCollection(b2.slice()); } else { // collection will be kept - collection = vocbase->createCollection(b2.slice()).get(); + collection = vocbase->createCollection(b2.slice()); } TRI_ASSERT(collection != nullptr); } catch (basics::Exception const& ex) { @@ -1335,11 +1330,10 @@ bool MMFilesWalRecoverState::ReplayMarker(MMFilesMarker const* marker, } // ignore any potential error returned by this call - arangodb::LogicalCollection* collection = - state->releaseCollection(collectionId); + auto collection = state->releaseCollection(collectionId); if (collection == nullptr) { - collection = vocbase->lookupCollection(collectionId).get(); + collection = vocbase->lookupCollection(collectionId); } if (collection != nullptr) { @@ -1547,7 +1541,7 @@ int MMFilesWalRecoverState::fillIndexes() { // release all collections for (auto it = openedCollections.begin(); it != openedCollections.end(); ++it) { - arangodb::LogicalCollection* collection = (*it).second; + std::shared_ptr& collection = (*it).second; auto physical = static_cast(collection->getPhysical()); TRI_ASSERT(physical != nullptr); diff --git a/arangod/MMFiles/MMFilesWalRecoverState.h b/arangod/MMFiles/MMFilesWalRecoverState.h index bce9c5662c..d69574c00c 100644 --- a/arangod/MMFiles/MMFilesWalRecoverState.h +++ b/arangod/MMFiles/MMFilesWalRecoverState.h @@ -125,7 +125,7 @@ struct MMFilesWalRecoverState { TRI_vocbase_t* releaseDatabase(TRI_voc_tick_t); /// @brief release a collection (so it can be dropped) - arangodb::LogicalCollection* releaseCollection(TRI_voc_cid_t); + std::shared_ptr releaseCollection(TRI_voc_cid_t); /// @brief gets a collection (and inserts it into the cache if not in it) arangodb::LogicalCollection* useCollection(TRI_vocbase_t*, TRI_voc_cid_t, int&); @@ -178,7 +178,8 @@ struct MMFilesWalRecoverState { TRI_voc_tick_t lastTick; std::vector logfilesToProcess; - std::unordered_map openedCollections; + std::unordered_map> openedCollections; std::unordered_map openedDatabases; std::vector emptyLogfiles; diff --git a/arangod/Replication/Syncer.cpp b/arangod/Replication/Syncer.cpp index ab65aa17ab..6e3aa2de76 100644 --- a/arangod/Replication/Syncer.cpp +++ b/arangod/Replication/Syncer.cpp @@ -766,9 +766,9 @@ Result Syncer::dropIndex(arangodb::velocypack::Slice const& slice) { return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND); } - auto* col = resolveCollection(*vocbase, slice).get(); + auto col = resolveCollection(*vocbase, slice); - if (col == nullptr) { + if (!col) { return Result(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); } diff --git a/arangod/Replication/TailingSyncer.cpp b/arangod/Replication/TailingSyncer.cpp index 97e4e8e11b..45cff5d94f 100644 --- a/arangod/Replication/TailingSyncer.cpp +++ b/arangod/Replication/TailingSyncer.cpp @@ -678,9 +678,9 @@ Result TailingSyncer::changeCollection(VPackSlice const& slice) { return Result(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND); } - auto* col = resolveCollection(*vocbase, slice).get(); + auto col = resolveCollection(*vocbase, slice); - if (col == nullptr) { + if (!col) { if (isDeleted) { // not a problem if a collection that is going to be deleted anyway // does not exist on slave diff --git a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp index 836c6406e1..b121cfbdf1 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationContext.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationContext.cpp @@ -105,7 +105,7 @@ void RocksDBReplicationContext::removeVocbase(TRI_vocbase_t& vocbase) { auto it = _iterators.begin(); while (it != _iterators.end()) { - if (it->second->dbGuard.database().id() == vocbase.id()) { + if (it->second->vocbase.id() == vocbase.id()) { if (it->second->isUsed()) { LOG_TOPIC(ERR, Logger::REPLICATION) << "trying to delete used context"; } else { @@ -736,7 +736,7 @@ void RocksDBReplicationContext::use(double ttl) { // make sure the WAL files are not deleted std::set dbs; for (auto& pair : _iterators) { - dbs.emplace(&pair.second->dbGuard.database()); + dbs.emplace(&pair.second->vocbase); } for (TRI_vocbase_t* vocbase : dbs) { vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl); @@ -754,7 +754,7 @@ void RocksDBReplicationContext::release() { // make sure the WAL files are not deleted immediately std::set dbs; for (auto& pair : _iterators) { - dbs.emplace(&pair.second->dbGuard.database()); + dbs.emplace(&pair.second->vocbase); } for (TRI_vocbase_t* vocbase : dbs) { vocbase->updateReplicationClient(replicationClientId(), _snapshotTick, ttl); @@ -782,7 +782,7 @@ RocksDBReplicationContext::CollectionIterator::CollectionIterator( TRI_vocbase_t& vocbase, std::shared_ptr const& coll, bool sorted, rocksdb::Snapshot const* snapshot) - : dbGuard(vocbase), + : vocbase(vocbase), logical{coll}, iter{nullptr}, bounds{RocksDBKeyBounds::Empty()}, @@ -797,17 +797,33 @@ RocksDBReplicationContext::CollectionIterator::CollectionIterator( _isUsed{false}, _sortedIterator{!sorted} // this makes sure that setSorted works { - TRI_ASSERT(snapshot != nullptr); + TRI_ASSERT(snapshot != nullptr && coll); _readOptions.snapshot = snapshot; _readOptions.verify_checksums = false; _readOptions.fill_cache = false; _readOptions.prefix_same_as_start = true; + if (!vocbase.use()) { // false if vobase was deleted + THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND); + } + TRI_vocbase_col_status_e ignore; + int res = vocbase.useCollection(logical.get(), ignore); + if (res != TRI_ERROR_NO_ERROR) { // collection was deleted + THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); + } + _cTypeHandler.reset(transaction::Context::createCustomTypeHandler(vocbase, _resolver)); vpackOptions.customTypeHandler = _cTypeHandler.get(); setSorted(sorted); } +RocksDBReplicationContext::CollectionIterator::~CollectionIterator() { + TRI_ASSERT(!vocbase.isDangling()); + vocbase.releaseCollection(logical.get()); + logical.reset(); + vocbase.release(); +} + void RocksDBReplicationContext::CollectionIterator::setSorted(bool sorted) { if (_sortedIterator != sorted) { iter.reset(); @@ -904,7 +920,7 @@ RocksDBReplicationContext::getCollectionIterator(TRI_vocbase_t& vocbase, } if (cIter) { - TRI_ASSERT(cIter->dbGuard.database().id() == vocbase.id()); + TRI_ASSERT(cIter->vocbase.id() == vocbase.id()); cIter->use(); if (allowCreate && cIter->sorted() != sorted) { @@ -915,7 +931,7 @@ RocksDBReplicationContext::getCollectionIterator(TRI_vocbase_t& vocbase, // for initial synchronization. the inventory request and collection // dump requests will all happen after the batch creation, so the // current tick value here is good - cIter->dbGuard.database().updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); + cIter->vocbase.updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); } return cIter; @@ -925,7 +941,7 @@ void RocksDBReplicationContext::releaseDumpIterator(CollectionIterator* it) { if (it) { TRI_ASSERT(it->isUsed()); if (!it->hasMore()) { - it->dbGuard.database().updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); + it->vocbase.updateReplicationClient(replicationClientId(), _snapshotTick, _ttl); MUTEX_LOCKER(locker, _contextLock); _iterators.erase(it->logical->id()); } else { // Context::release() will update the replication client diff --git a/arangod/RocksDBEngine/RocksDBReplicationContext.h b/arangod/RocksDBEngine/RocksDBReplicationContext.h index 25d261cb6f..e35cfb5537 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationContext.h +++ b/arangod/RocksDBEngine/RocksDBReplicationContext.h @@ -32,7 +32,6 @@ #include "RocksDBEngine/RocksDBReplicationCommon.h" #include "Transaction/Methods.h" #include "Utils/CollectionNameResolver.h" -#include "Utils/DatabaseGuard.h" #include "VocBase/vocbase.h" #include @@ -49,7 +48,6 @@ namespace rocksdb { } namespace arangodb { -class DatabaseGuard; class RocksDBReplicationContext { private: @@ -62,8 +60,9 @@ class RocksDBReplicationContext { CollectionIterator(TRI_vocbase_t&, std::shared_ptr const&, bool sorted, rocksdb::Snapshot const*); + ~CollectionIterator(); - DatabaseGuard dbGuard; + TRI_vocbase_t& vocbase; std::shared_ptr logical; /// Iterator over primary index or documents diff --git a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp index 154af11d9b..61b6bbc54e 100644 --- a/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp +++ b/arangod/RocksDBEngine/RocksDBReplicationTailing.cpp @@ -594,11 +594,10 @@ class WALParser final : public rocksdb::WriteBatch::Handler { return it->second.collection(); } - auto* collection = _vocbase->lookupCollection(cid).get(); - - if (collection != nullptr) { + auto collection = _vocbase->lookupCollection(cid); + if (collection) { _collectionCache.emplace(cid, CollectionGuard(_vocbase, collection)); - return collection; + return collection.get(); } } diff --git a/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp b/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp index a9fdeeb265..931bf9cbda 100644 --- a/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp +++ b/arangod/RocksDBEngine/RocksDBTransactionCollection.cpp @@ -194,7 +194,7 @@ int RocksDBTransactionCollection::use(int nestingLevel) { _usageLocked = true; } else { // use without usage-lock (lock already set externally) - _collection = _transaction->vocbase().lookupCollection(_cid).get(); + _collection = _transaction->vocbase().lookupCollection(_cid); if (_collection == nullptr) { return TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND; @@ -246,7 +246,7 @@ void RocksDBTransactionCollection::release() { LOG_TRX(_transaction, 0) << "unusing collection " << _cid; if (_usageLocked) { - _transaction->vocbase().releaseCollection(_collection); + _transaction->vocbase().releaseCollection(_collection.get()); _usageLocked = false; } @@ -390,10 +390,7 @@ int RocksDBTransactionCollection::doLock(AccessMode::Type type, TRI_ASSERT(!isLocked()); - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); - - auto physical = static_cast(collection->getPhysical()); + auto physical = static_cast(_collection->getPhysical()); TRI_ASSERT(physical != nullptr); double timeout = _transaction->timeout(); @@ -473,10 +470,9 @@ int RocksDBTransactionCollection::doUnlock(AccessMode::Type type, return TRI_ERROR_INTERNAL; } - LogicalCollection* collection = _collection; - TRI_ASSERT(collection != nullptr); + TRI_ASSERT(_collection); - auto physical = static_cast(collection->getPhysical()); + auto physical = static_cast(_collection->getPhysical()); TRI_ASSERT(physical != nullptr); LOG_TRX(_transaction, nestingLevel) << "write-unlocking collection " << _cid; diff --git a/arangod/StorageEngine/TransactionCollection.h b/arangod/StorageEngine/TransactionCollection.h index cf681ac3e6..1c1ecc4fbe 100644 --- a/arangod/StorageEngine/TransactionCollection.h +++ b/arangod/StorageEngine/TransactionCollection.h @@ -50,7 +50,7 @@ class TransactionCollection { inline TRI_voc_cid_t id() const { return _cid; } LogicalCollection* collection() const { - return _collection; // vocbase collection pointer + return _collection.get(); // vocbase collection pointer } std::string collectionName() const; @@ -91,10 +91,10 @@ class TransactionCollection { virtual void release() = 0; protected: - TransactionState* _transaction; // the transaction state - TRI_voc_cid_t const _cid; // collection id - LogicalCollection* _collection; // vocbase collection pointer - AccessMode::Type _accessType; // access type (read|write) + TransactionState* _transaction; // the transaction state + TRI_voc_cid_t const _cid; // collection id + std::shared_ptr _collection; // vocbase collection pointer + AccessMode::Type _accessType; // access type (read|write) }; } diff --git a/arangod/Utils/CollectionGuard.h b/arangod/Utils/CollectionGuard.h index b62e8f4c4e..ab89807e5f 100644 --- a/arangod/Utils/CollectionGuard.h +++ b/arangod/Utils/CollectionGuard.h @@ -47,13 +47,13 @@ class CollectionGuard { } /// @brief create the guard, using a collection id - CollectionGuard(TRI_vocbase_t* vocbase, TRI_voc_cid_t id, + CollectionGuard(TRI_vocbase_t* vocbase, TRI_voc_cid_t cid, bool restoreOriginalStatus = false) : _vocbase(vocbase), _collection(nullptr), _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _restoreOriginalStatus(restoreOriginalStatus) { - _collection = _vocbase->useCollection(id, _originalStatus); + _collection = _vocbase->useCollection(cid, _originalStatus); if (_collection == nullptr) { THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); @@ -67,11 +67,9 @@ class CollectionGuard { _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _restoreOriginalStatus(false) { _collection = _vocbase->useCollection(id, _originalStatus); - - if (_collection == nullptr && !name.empty()) { + if (!_collection && !name.empty()) { _collection = _vocbase->useCollection(name, _originalStatus); } - if (_collection == nullptr) { THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); } @@ -96,12 +94,13 @@ class CollectionGuard { } } - CollectionGuard(TRI_vocbase_t* vocbase, LogicalCollection* collection) + CollectionGuard(TRI_vocbase_t* vocbase, + std::shared_ptr const& collection) : _vocbase(vocbase), _collection(collection), _originalStatus(TRI_VOC_COL_STATUS_CORRUPTED), _restoreOriginalStatus(false) { - int res = _vocbase->useCollection(collection, _originalStatus); + int res = _vocbase->useCollection(collection.get(), _originalStatus); if (res != TRI_ERROR_NO_ERROR) { THROW_ARANGO_EXCEPTION(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); } @@ -110,13 +109,13 @@ class CollectionGuard { /// @brief destroy the guard ~CollectionGuard() { if (_collection != nullptr) { - _vocbase->releaseCollection(_collection); + _vocbase->releaseCollection(_collection.get()); if (_restoreOriginalStatus && (_originalStatus == TRI_VOC_COL_STATUS_UNLOADING || _originalStatus == TRI_VOC_COL_STATUS_UNLOADED)) { // re-unload the collection - _vocbase->unloadCollection(_collection, false); + _vocbase->unloadCollection(_collection.get(), false); } } } @@ -125,13 +124,13 @@ class CollectionGuard { /// @brief prematurely release the usage lock void release() { if (_collection != nullptr) { - _vocbase->releaseCollection(_collection); + _vocbase->releaseCollection(_collection.get()); _collection = nullptr; } } /// @brief return the collection pointer - inline arangodb::LogicalCollection* collection() const { return _collection; } + inline arangodb::LogicalCollection* collection() const { return _collection.get(); } /// @brief return the status of the collection at the time of using the guard inline TRI_vocbase_col_status_e originalStatus() const { @@ -143,7 +142,7 @@ class CollectionGuard { TRI_vocbase_t* _vocbase; /// @brief pointer to collection - arangodb::LogicalCollection* _collection; + std::shared_ptr _collection; /// @brief status of collection when invoking the guard TRI_vocbase_col_status_e _originalStatus; diff --git a/arangod/VocBase/vocbase.cpp b/arangod/VocBase/vocbase.cpp index 25cb03f4b1..073e7c0662 100644 --- a/arangod/VocBase/vocbase.cpp +++ b/arangod/VocBase/vocbase.cpp @@ -1552,56 +1552,48 @@ int TRI_vocbase_t::useCollection(arangodb::LogicalCollection* collection, } /// @brief locks a (document) collection for usage by id -arangodb::LogicalCollection* TRI_vocbase_t::useCollection( +std::shared_ptr TRI_vocbase_t::useCollection( TRI_voc_cid_t cid, TRI_vocbase_col_status_e& status) { - auto collection = lookupCollection(cid); - - return useCollectionInternal(collection.get(), status); + return useCollectionInternal(lookupCollection(cid), status); } /// @brief locks a collection for usage by name -arangodb::LogicalCollection* TRI_vocbase_t::useCollection( +std::shared_ptr TRI_vocbase_t::useCollection( std::string const& name, TRI_vocbase_col_status_e& status) { // check that we have an existing name - arangodb::LogicalCollection* collection = nullptr; - + std::shared_ptr collection; { RECURSIVE_READ_LOCKER(_dataSourceLock, _dataSourceLockWriteOwner); auto it = _dataSourceByName.find(name); - if (it != _dataSourceByName.end() && it->second->category() == LogicalCollection::category()) { TRI_ASSERT(std::dynamic_pointer_cast(it->second)); - collection = static_cast(it->second.get()); + collection = std::static_pointer_cast(it->second); } } - return useCollectionInternal(collection, status); + return useCollectionInternal(std::move(collection), status); } /// @brief locks a collection for usage by name -arangodb::LogicalCollection* TRI_vocbase_t::useCollectionByUuid( +std::shared_ptr TRI_vocbase_t::useCollectionByUuid( std::string const& uuid, TRI_vocbase_col_status_e& status) { - auto collection = lookupCollectionByUuid(uuid); - - return useCollectionInternal(collection.get(), status); + return useCollectionInternal(lookupCollectionByUuid(uuid), status); } -arangodb::LogicalCollection* TRI_vocbase_t::useCollectionInternal( - arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status) { - if (collection == nullptr) { +std::shared_ptr TRI_vocbase_t::useCollectionInternal( + std::shared_ptr coll, TRI_vocbase_col_status_e& status) { + if (!coll) { TRI_set_errno(TRI_ERROR_ARANGO_DATA_SOURCE_NOT_FOUND); return nullptr; } // try to load the collection - int res = loadCollection(collection, status); - + int res = loadCollection(coll.get(), status); if (res == TRI_ERROR_NO_ERROR) { - return collection; + return coll; } - TRI_set_errno(res); return nullptr; } diff --git a/arangod/VocBase/vocbase.h b/arangod/VocBase/vocbase.h index 8e59687b66..ac6f924f32 100644 --- a/arangod/VocBase/vocbase.h +++ b/arangod/VocBase/vocbase.h @@ -370,22 +370,22 @@ struct TRI_vocbase_t { /// Note that this will READ lock the collection you have to release the /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// when you are done with the collection. - arangodb::LogicalCollection* useCollection(TRI_voc_cid_t cid, - TRI_vocbase_col_status_e&); + std::shared_ptr useCollection(TRI_voc_cid_t cid, + TRI_vocbase_col_status_e&); /// @brief locks a collection for usage by name /// Note that this will READ lock the collection you have to release the /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// when you are done with the collection. - arangodb::LogicalCollection* useCollection(std::string const& name, - TRI_vocbase_col_status_e&); + std::shared_ptr useCollection(std::string const& name, + TRI_vocbase_col_status_e&); /// @brief locks a collection for usage by uuid /// Note that this will READ lock the collection you have to release the /// collection lock by yourself and call @ref TRI_ReleaseCollectionVocBase /// when you are done with the collection. - arangodb::LogicalCollection* useCollectionByUuid(std::string const& uuid, - TRI_vocbase_col_status_e&); + std::shared_ptr useCollectionByUuid(std::string const& uuid, + TRI_vocbase_col_status_e&); /// @brief releases a collection from usage void releaseCollection(arangodb::LogicalCollection* collection); @@ -405,8 +405,8 @@ struct TRI_vocbase_t { /// @brief check some invariants on the various lists of collections void checkCollectionInvariants() const; - arangodb::LogicalCollection* useCollectionInternal( - arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status); + std::shared_ptr useCollectionInternal( + std::shared_ptr, TRI_vocbase_col_status_e& status); int loadCollection(arangodb::LogicalCollection* collection, TRI_vocbase_col_status_e& status, bool setStatus = true); @@ -455,4 +455,4 @@ void TRI_SanitizeObject(arangodb::velocypack::Slice const slice, void TRI_SanitizeObjectWithEdges(arangodb::velocypack::Slice const slice, arangodb::velocypack::Builder& builder); -#endif \ No newline at end of file +#endif diff --git a/tests/IResearch/StorageEngineMock.cpp b/tests/IResearch/StorageEngineMock.cpp index c0cf7125b5..2c6e690af8 100644 --- a/tests/IResearch/StorageEngineMock.cpp +++ b/tests/IResearch/StorageEngineMock.cpp @@ -1447,7 +1447,7 @@ int TransactionCollectionMock::lockRecursive(arangodb::AccessMode::Type type, in void TransactionCollectionMock::release() { if (_collection) { - _transaction->vocbase().releaseCollection(_collection); + _transaction->vocbase().releaseCollection(_collection.get()); _collection = nullptr; } }