Skip to content

Commit

Permalink
SERVER-21311 fixing crash on shutdown in RocksRecoveryUnit::dbRelease…
Browse files Browse the repository at this point in the history
…Snapshot

Problem description:
The crash was caused by dangling pointer to RocksRecoveryUnit stored in the instance of RocksSnapshotManager::SnapshotHolder class.

Crash scenario:
- execute administrative 'makeSnapshot' command. This will create instance of RocksSnapshotManager::SnapshotHolder which is stored in RocksSnapshotManager's snapshots map.
- SnapshotHolder has raw pointer to RocksRecoveryUnit instance which is destroyed at the end of 'makeSnapshot' command handler
- destructor of SnapshotHolder is called on shutdown when snapshot maps are cleared in RocksSnapshotManager::dropAllSnapshots
- destructor of SnapshotHolder tries to release snapshot using stored pointer to RocksRecoveryUnit instance which was destroyed earlier

Fix description:
Instead of storing RocksRecoveryUnit pointer in SnapshotHolder we will store pointer to rocksdb::DB instance.
Thus we will be able to call rocks::DB::ReleaseSnapshot() from SnapshotHolder's destructor and SnapshotHolder
will be independent of RocksRecoveryUnit lifetime.
  • Loading branch information
igorsol committed Nov 20, 2015
1 parent db7b804 commit ba114dc
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
2 changes: 2 additions & 0 deletions src/rocks_recovery_unit.h
Expand Up @@ -160,6 +160,8 @@ namespace mongo {

void setCommittedSnapshot(const rocksdb::Snapshot* committedSnapshot);

rocksdb::DB* getDB() const { return _db; }

private:
void _releaseSnapshot();

Expand Down
9 changes: 5 additions & 4 deletions src/rocks_snapshot_manager.cpp
Expand Up @@ -102,14 +102,15 @@ namespace mongo {

RocksSnapshotManager::SnapshotHolder::SnapshotHolder(OperationContext* opCtx, uint64_t name_) {
name = name_;
ru = RocksRecoveryUnit::getRocksRecoveryUnit(opCtx);
snapshot = ru->getPreparedSnapshot();
auto rru = RocksRecoveryUnit::getRocksRecoveryUnit(opCtx);
snapshot = rru->getPreparedSnapshot();
db = rru->getDB();
}

RocksSnapshotManager::SnapshotHolder::~SnapshotHolder() {
if (snapshot != nullptr) {
invariant(ru != nullptr);
ru->dbReleaseSnapshot(snapshot);
invariant(db != nullptr);
db->ReleaseSnapshot(snapshot);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/rocks_snapshot_manager.h
Expand Up @@ -48,7 +48,7 @@ class RocksSnapshotManager final : public SnapshotManager {
struct SnapshotHolder {
uint64_t name;
const rocksdb::Snapshot* snapshot;
RocksRecoveryUnit* ru;
rocksdb::DB* db;
SnapshotHolder(OperationContext* opCtx, uint64_t name_);
~SnapshotHolder();
};
Expand Down

0 comments on commit ba114dc

Please sign in to comment.