SERVER-33366 OperationContext setRecoveryUnit() & releaseRecoveryUnit() should transfer using unique_ptr.

13 files changed, 41 insertions, 33 deletions

diff --git a/src/mongo/db/auth/authorization_manager_test.cpp b/src/mongo/db/auth/authorization_manager_test.cpp
index dc3701e2ecf..7e93729aed3 100644
--- a/src/mongo/db/auth/authorization_manager_test.cpp
+++ b/src/mongo/db/auth/authorization_manager_test.cpp
@@ -352,7 +352,8 @@ public:
     };
 
     virtual void setUp() override {
-        opCtx->setRecoveryUnit(recoveryUnit, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
+        opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recoveryUnit),
+                               WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         AuthorizationManagerTest::setUp();
     }
 
diff --git a/src/mongo/db/concurrency/d_concurrency_test.cpp b/src/mongo/db/concurrency/d_concurrency_test.cpp
index 2738a8db2e6..df07a688104 100644
--- a/src/mongo/db/concurrency/d_concurrency_test.cpp
+++ b/src/mongo/db/concurrency/d_concurrency_test.cpp
@@ -1658,7 +1658,7 @@ TEST_F(DConcurrencyTestFixture, TestGlobalLockAbandonsSnapshotWhenNotInWriteUnit
     auto opCtx = clients[0].second.get();
     auto recovUnitOwned = stdx::make_unique<RecoveryUnitMock>();
     auto recovUnitBorrowed = recovUnitOwned.get();
-    opCtx->setRecoveryUnit(recovUnitOwned.release(),
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recovUnitOwned.release()),
                            WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 
     {
@@ -1683,7 +1683,7 @@ TEST_F(DConcurrencyTestFixture, TestGlobalLockDoesNotAbandonSnapshotWhenInWriteU
     auto opCtx = clients[0].second.get();
     auto recovUnitOwned = stdx::make_unique<RecoveryUnitMock>();
     auto recovUnitBorrowed = recovUnitOwned.get();
-    opCtx->setRecoveryUnit(recovUnitOwned.release(),
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recovUnitOwned.release()),
                            WriteUnitOfWork::RecoveryUnitState::kActiveUnitOfWork);
     opCtx->lockState()->beginWriteUnitOfWork();
 
diff --git a/src/mongo/db/operation_context.cpp b/src/mongo/db/operation_context.cpp
index 091ffdd1ee4..94a5ea312d2 100644
--- a/src/mongo/db/operation_context.cpp
+++ b/src/mongo/db/operation_context.cpp
@@ -379,13 +379,13 @@ void OperationContext::setTxnNumber(TxnNumber txnNumber) {
     _txnNumber = txnNumber;
 }
 
-RecoveryUnit* OperationContext::releaseRecoveryUnit() {
-    return _recoveryUnit.release();
+std::unique_ptr<RecoveryUnit> OperationContext::releaseRecoveryUnit() {
+    return std::move(_recoveryUnit);
 }
 
 WriteUnitOfWork::RecoveryUnitState OperationContext::setRecoveryUnit(
-    RecoveryUnit* unit, WriteUnitOfWork::RecoveryUnitState state) {
-    _recoveryUnit.reset(unit);
+    std::unique_ptr<RecoveryUnit> unit, WriteUnitOfWork::RecoveryUnitState state) {
+    _recoveryUnit = std::move(unit);
     WriteUnitOfWork::RecoveryUnitState oldState = _ruState;
     _ruState = state;
     return oldState;
diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h
index 96c2fecc76e..712a0ab860c 100644
--- a/src/mongo/db/operation_context.h
+++ b/src/mongo/db/operation_context.h
@@ -97,7 +97,7 @@ public:
      * We rely on active cursors being killed when collections or databases are dropped,
      * or when collection metadata changes.
      */
-    RecoveryUnit* releaseRecoveryUnit();
+    std::unique_ptr<RecoveryUnit> releaseRecoveryUnit();
 
     /**
      * Associates the OperatingContext with a different RecoveryUnit for getMore or
@@ -105,7 +105,7 @@ public:
      * returned separately even though the state logically belongs to the RecoveryUnit,
      * as it is managed by the OperationContext.
      */
-    WriteUnitOfWork::RecoveryUnitState setRecoveryUnit(RecoveryUnit* unit,
+    WriteUnitOfWork::RecoveryUnitState setRecoveryUnit(std::unique_ptr<RecoveryUnit> unit,
                                                        WriteUnitOfWork::RecoveryUnitState state);
 
     /**
diff --git a/src/mongo/db/operation_context_noop.h b/src/mongo/db/operation_context_noop.h
index 1f4e51c40f5..dc21cd829bd 100644
--- a/src/mongo/db/operation_context_noop.h
+++ b/src/mongo/db/operation_context_noop.h
@@ -46,14 +46,15 @@ public:
      */
     OperationContextNoop() : OperationContextNoop(nullptr, 0) {}
     OperationContextNoop(RecoveryUnit* ru) : OperationContextNoop(nullptr, 0) {
-        setRecoveryUnit(ru, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
+        setRecoveryUnit(std::unique_ptr<RecoveryUnit>(ru),
+                        WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
     }
 
     /**
      * This constructor is for use by ServiceContexts, and should not be called directly.
      */
     OperationContextNoop(Client* client, unsigned int opId) : OperationContext(client, opId) {
-        setRecoveryUnit(new RecoveryUnitNoop(),
+        setRecoveryUnit(std::make_unique<RecoveryUnitNoop>(),
                         WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         setLockState(std::make_unique<LockerNoop>());
     }
diff --git a/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp b/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp
index d20a8d64423..92f95a35f82 100644
--- a/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp
+++ b/src/mongo/db/repl/replication_consistency_markers_impl_test.cpp
@@ -272,7 +272,8 @@ TEST_F(ReplicationConsistencyMarkersTest, ReplicationConsistencyMarkers) {
 
     // Recovery unit will be owned by "opCtx".
     RecoveryUnitWithDurabilityTracking* recoveryUnit = new RecoveryUnitWithDurabilityTracking();
-    opCtx->setRecoveryUnit(recoveryUnit, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(recoveryUnit),
+                           WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 
     // Set min valid without waiting for the changes to be durable.
     OpTime endOpTime2({Seconds(789), 0}, 1LL);
diff --git a/src/mongo/db/service_context.cpp b/src/mongo/db/service_context.cpp
index 038cb0a67c4..b14d9332e2b 100644
--- a/src/mongo/db/service_context.cpp
+++ b/src/mongo/db/service_context.cpp
@@ -235,7 +235,7 @@ ServiceContext::UniqueOperationContext ServiceContext::makeOperationContext(Clie
         opCtx->setLockState(std::make_unique<LockerNoop>());
     }
     if (!opCtx->recoveryUnit()) {
-        opCtx->setRecoveryUnit(new RecoveryUnitNoop(),
+        opCtx->setRecoveryUnit(std::make_unique<RecoveryUnitNoop>(),
                                WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
     }
     {
diff --git a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
index 90c51d942c8..82cb77e8b8b 100644
--- a/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
+++ b/src/mongo/db/storage/kv/kv_engine_test_timestamps.cpp
@@ -56,8 +56,9 @@ public:
         Operation() = default;
         Operation(ServiceContext::UniqueClient client, RecoveryUnit* ru)
             : _client(std::move(client)), _opCtx(_client->makeOperationContext()) {
-            delete _opCtx->releaseRecoveryUnit();
-            _opCtx->setRecoveryUnit(ru, WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
+            _opCtx->releaseRecoveryUnit();
+            _opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(ru),
+                                    WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         }
 
 
diff --git a/src/mongo/db/storage/storage_engine_init.cpp b/src/mongo/db/storage/storage_engine_init.cpp
index 4805b8105ee..085de0e01c4 100644
--- a/src/mongo/db/storage/storage_engine_init.cpp
+++ b/src/mongo/db/storage/storage_engine_init.cpp
@@ -330,7 +330,7 @@ public:
             return;
         }
         opCtx->setLockState(stdx::make_unique<LockerImpl>());
-        opCtx->setRecoveryUnit(storageEngine->newRecoveryUnit(),
+        opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(storageEngine->newRecoveryUnit()),
                                WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
     }
     void onDestroyOperationContext(OperationContext* opCtx) {}
diff --git a/src/mongo/db/storage/test_harness_helper.h b/src/mongo/db/storage/test_harness_helper.h
index e21947e917c..8513ee6c70b 100644
--- a/src/mongo/db/storage/test_harness_helper.h
+++ b/src/mongo/db/storage/test_harness_helper.h
@@ -59,7 +59,7 @@ public:
 
     virtual ServiceContext::UniqueOperationContext newOperationContext(Client* const client) {
         auto opCtx = client->makeOperationContext();
-        opCtx->setRecoveryUnit(newRecoveryUnit().release(),
+        opCtx->setRecoveryUnit(newRecoveryUnit(),
                                WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         return opCtx;
     }
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
index e5b1e785527..09b42c028ec 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_record_store.cpp
@@ -1016,11 +1016,12 @@ int64_t WiredTigerRecordStore::_cappedDeleteAsNeeded_inlock(OperationContext* op
                                                             const RecordId& justInserted) {
     // we do this in a side transaction in case it aborts
     WiredTigerRecoveryUnit* realRecoveryUnit =
-        checked_cast<WiredTigerRecoveryUnit*>(opCtx->releaseRecoveryUnit());
+        checked_cast<WiredTigerRecoveryUnit*>(opCtx->releaseRecoveryUnit().release());
     invariant(realRecoveryUnit);
     WiredTigerSessionCache* sc = realRecoveryUnit->getSessionCache();
-    WriteUnitOfWork::RecoveryUnitState const realRUstate = opCtx->setRecoveryUnit(
-        new WiredTigerRecoveryUnit(sc), WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
+    WriteUnitOfWork::RecoveryUnitState const realRUstate =
+        opCtx->setRecoveryUnit(std::make_unique<WiredTigerRecoveryUnit>(sc),
+                               WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 
     WT_SESSION* session = WiredTigerRecoveryUnit::get(opCtx)->getSession()->getSession();
 
@@ -1148,18 +1149,18 @@ int64_t WiredTigerRecordStore::_cappedDeleteAsNeeded_inlock(OperationContext* op
             }
         }
     } catch (const WriteConflictException&) {
-        delete opCtx->releaseRecoveryUnit();
-        opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate);
+        opCtx->releaseRecoveryUnit();
+        opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate);
         log() << "got conflict truncating capped, ignoring";
         return 0;
     } catch (...) {
-        delete opCtx->releaseRecoveryUnit();
-        opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate);
+        opCtx->releaseRecoveryUnit();
+        opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate);
         throw;
     }
 
-    delete opCtx->releaseRecoveryUnit();
-    opCtx->setRecoveryUnit(realRecoveryUnit, realRUstate);
+    opCtx->releaseRecoveryUnit();
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(realRecoveryUnit), realRUstate);
     return docsRemoved;
 }
 
diff --git a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp
index 0c3f74516f9..f5c0688dd15 100644
--- a/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp
+++ b/src/mongo/db/storage/wiredtiger/wiredtiger_recovery_unit_test.cpp
@@ -134,7 +134,7 @@ public:
         auto sc = harnessHelper->serviceContext();
         auto client = sc->makeClient(clientName);
         auto opCtx = client->makeOperationContext();
-        opCtx->setRecoveryUnit(harnessHelper->newRecoveryUnit().release(),
+        opCtx->setRecoveryUnit(harnessHelper->newRecoveryUnit(),
                                WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         return std::make_pair(std::move(client), std::move(opCtx));
     }
diff --git a/src/mongo/db/transaction_participant.cpp b/src/mongo/db/transaction_participant.cpp
index 1fa3dcd9b72..aefac7c9c5f 100644
--- a/src/mongo/db/transaction_participant.cpp
+++ b/src/mongo/db/transaction_participant.cpp
@@ -248,8 +248,9 @@ TransactionParticipant::OplogSlotReserver::OplogSlotReserver(OperationContext* o
     }
 
     // Save the RecoveryUnit from the new transaction and replace it with an empty one.
-    _recoveryUnit = std::unique_ptr<RecoveryUnit>(opCtx->releaseRecoveryUnit());
-    opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(),
+    _recoveryUnit = opCtx->releaseRecoveryUnit();
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(
+                               opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()),
                            WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 }
 
@@ -281,8 +282,9 @@ TransactionParticipant::TxnResources::TxnResources(OperationContext* opCtx, bool
         opCtx->lockState()->setMaxLockTimeout(Milliseconds(maxTransactionLockMillis));
     }
 
-    _recoveryUnit = std::unique_ptr<RecoveryUnit>(opCtx->releaseRecoveryUnit());
-    opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(),
+    _recoveryUnit = opCtx->releaseRecoveryUnit();
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(
+                               opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()),
                            WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 
     _readConcernArgs = repl::ReadConcernArgs::get(opCtx);
@@ -313,7 +315,7 @@ void TransactionParticipant::TxnResources::release(OperationContext* opCtx) {
     opCtx->swapLockState(std::move(_locker));
     opCtx->lockState()->updateThreadIdToCurrentThread();
 
-    auto oldState = opCtx->setRecoveryUnit(_recoveryUnit.release(),
+    auto oldState = opCtx->setRecoveryUnit(std::move(_recoveryUnit),
                                            WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
     invariant(oldState == WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork,
               str::stream() << "RecoveryUnit state was " << oldState);
@@ -858,7 +860,8 @@ void TransactionParticipant::_cleanUpTxnResourceOnOpCtx(WithLock wl, OperationCo
 
     // We must clear the recovery unit and locker so any post-transaction writes can run without
     // transactional settings such as a read timestamp.
-    opCtx->setRecoveryUnit(opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit(),
+    opCtx->setRecoveryUnit(std::unique_ptr<RecoveryUnit>(
+                               opCtx->getServiceContext()->getStorageEngine()->newRecoveryUnit()),
                            WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
 
     opCtx->lockState()->unsetMaxLockTimeout();