SERVER-86869 Dont call getNewSession() as a parameter of a function (#27106)

GitOrigin-RevId: 1dabc0b93b9a8537172bb649fae552381964fe0b

9 files changed, 111 insertions, 78 deletions

diff --git a/src/mongo/db/s/convert_to_capped_coordinator.cpp b/src/mongo/db/s/convert_to_capped_coordinator.cpp
index 281bc4e32c7..5ab20bf2ab4 100644
--- a/src/mongo/db/s/convert_to_capped_coordinator.cpp
+++ b/src/mongo/db/s/convert_to_capped_coordinator.cpp
@@ -312,15 +312,18 @@ ExecutorFuture<void> ConvertToCappedCoordinator::_runImpl(
                     uasserted(ErrorCodes::InternalError,
                               "Reproducing an error. This is part of a test.");
                 }
-                const auto& session = getNewSession(opCtx);
-                convertToCappedOnShard(opCtx,
-                                       nss(),
-                                       _doc.getSize(),
-                                       *_doc.getDataShard(),
-                                       *_doc.getTargetUUID(),
-                                       session,
-                                       executor,
-                                       token);
+
+                {
+                    const auto session = getNewSession(opCtx);
+                    convertToCappedOnShard(opCtx,
+                                           nss(),
+                                           _doc.getSize(),
+                                           *_doc.getDataShard(),
+                                           *_doc.getTargetUUID(),
+                                           session,
+                                           executor,
+                                           token);
+                }
 
                 if (MONGO_unlikely(convertToCappedFailAfterCappingTheCollection.shouldFail())) {
                     convertToCappedFailAfterCappingTheCollection.pauseWhileSet();
@@ -366,16 +369,19 @@ ExecutorFuture<void> ConvertToCappedCoordinator::_runImpl(
                     // guarantee targeting the config server
                     const bool useClusterTransaction{true};
 
-                    // Delete the sharding catalog entries referring the previous incarnation
-                    sharding_ddl_util::removeCollAndChunksMetadataFromConfig(
-                        opCtx,
-                        Grid::get(opCtx)->shardRegistry()->getConfigShard(),
-                        Grid::get(opCtx)->catalogClient(),
-                        *_doc.getOriginalCollection(),
-                        ShardingCatalogClient::kMajorityWriteConcern,
-                        getNewSession(opCtx),
-                        useClusterTransaction,
-                        **executor);
+                    {
+                        const auto session = getNewSession(opCtx);
+                        // Delete the sharding catalog entries referring the previous incarnation
+                        sharding_ddl_util::removeCollAndChunksMetadataFromConfig(
+                            opCtx,
+                            Grid::get(opCtx)->shardRegistry()->getConfigShard(),
+                            Grid::get(opCtx)->catalogClient(),
+                            *_doc.getOriginalCollection(),
+                            ShardingCatalogClient::kMajorityWriteConcern,
+                            session,
+                            useClusterTransaction,
+                            **executor);
+                    }
 
                     auto createCollectionOnShardingCatalogOps = sharding_ddl_util::
                         getOperationsToCreateUnsplittableCollectionOnShardingCatalog(
diff --git a/src/mongo/db/s/create_collection_coordinator.cpp b/src/mongo/db/s/create_collection_coordinator.cpp
index 93358299728..72f54fec1c2 100644
--- a/src/mongo/db/s/create_collection_coordinator.cpp
+++ b/src/mongo/db/s/create_collection_coordinator.cpp
@@ -1708,8 +1708,9 @@ ExecutorFuture<void> CreateCollectionCoordinatorLegacy::_runImpl(
                     std::vector<ShardId>{std::make_move_iterator(involvedShards.begin()),
                                          std::make_move_iterator(involvedShards.end())};
                 std::erase(participants, ShardingState::get(opCtx)->shardId());
+                const auto session = getNewSession(opCtx);
                 createCollectionOnShards(opCtx,
-                                         getNewSession(opCtx),
+                                         session,
                                          _collectionUUID,
                                          participants,
                                          nss(),
@@ -2260,7 +2261,7 @@ void CreateCollectionCoordinator::_syncIndexesOnCoordinator(
     _uuid = *sharding_ddl_util::getCollectionUUID(opCtx, nss());
 
     // Get indexes from the dataShard and copy them to the coordinator.
-    auto session = getNewSession(opCtx);
+    const auto session = getNewSession(opCtx);
     createCollectionOnShards(opCtx,
                              session,
                              _uuid,
@@ -2568,13 +2569,14 @@ void CreateCollectionCoordinator::_setPostCommitMetadata(
         const auto primaryShardId = ShardingState::get(opCtx)->shardId();
         std::erase(nonInvolvedShardIds, primaryShardId);
 
+        const auto session = getNewSession(opCtx);
         if (!nonInvolvedShardIds.empty()) {
             sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
                 opCtx,
                 nss(),
                 nonInvolvedShardIds,
                 **executor,
-                getNewSession(opCtx),
+                session,
                 true /* fromMigrate */,
                 false /* dropSystemCollections */,
                 _uuid);
@@ -2665,11 +2667,12 @@ ExecutorFuture<void> CreateCollectionCoordinator::_cleanupOnAbort(
                 // TODO SERVER-83774: Remove the following invariant and skip the broadcast if the
                 // _uuid does not exist.
                 invariant(_uuid);
+                const auto session = getNewSession(opCtx);
                 broadcastDropCollection(opCtx,
                                         nss(),
                                         *_doc.getOriginalDataShard() /* excludedDataShard */,
                                         **executor,
-                                        getNewSession(opCtx),
+                                        session,
                                         _uuid);
             }
 
diff --git a/src/mongo/db/s/drop_collection_coordinator.cpp b/src/mongo/db/s/drop_collection_coordinator.cpp
index 301e2b4ca4c..fc3e1fecb84 100644
--- a/src/mongo/db/s/drop_collection_coordinator.cpp
+++ b/src/mongo/db/s/drop_collection_coordinator.cpp
@@ -263,7 +263,8 @@ void DropCollectionCoordinator::_freezeMigrations(
 
     if (_doc.getCollInfo()) {
         const auto collUUID = _doc.getCollInfo()->getUuid();
-        sharding_ddl_util::stopMigrations(opCtx, nss(), collUUID, getNewSession(opCtx));
+        const auto session = getNewSession(opCtx);
+        sharding_ddl_util::stopMigrations(opCtx, nss(), collUUID, session);
     }
 }
 
@@ -325,7 +326,10 @@ void DropCollectionCoordinator::_commitDropCollection(
     }
 
     // Remove tags even if the collection is not sharded or didn't exist
-    sharding_ddl_util::removeTagsMetadataFromConfig(opCtx, nss(), getNewSession(opCtx));
+    {
+        const auto session = getNewSession(opCtx);
+        sharding_ddl_util::removeTagsMetadataFromConfig(opCtx, nss(), session);
+    }
 
     // Checkpoint the configTime to ensure that, in the case of a stepdown, the new primary will
     // start-up from a configTime that is inclusive of the metadata removable that was committed
@@ -341,26 +345,32 @@ void DropCollectionCoordinator::_commitDropCollection(
     participants.erase(std::remove(participants.begin(), participants.end(), primaryShardId),
                        participants.end());
 
-    sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
-        opCtx,
-        nss(),
-        participants,
-        **executor,
-        getNewSession(opCtx),
-        true /* fromMigrate */,
-        false /* dropSystemCollections */);
+    {
+        const auto session = getNewSession(opCtx);
+        sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
+            opCtx,
+            nss(),
+            participants,
+            **executor,
+            session,
+            true /* fromMigrate */,
+            false /* dropSystemCollections */);
+    }
 
     // The sharded collection must be dropped on the primary shard after it has been
     // dropped on all of the other shards to ensure it can only be re-created as
     // unsharded with a higher optime than all of the drops.
-    sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
-        opCtx,
-        nss(),
-        {primaryShardId},
-        **executor,
-        getNewSession(opCtx),
-        false /* fromMigrate */,
-        false /* dropSystemCollections */);
+    {
+        const auto session = getNewSession(opCtx);
+        sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
+            opCtx,
+            nss(),
+            {primaryShardId},
+            **executor,
+            session,
+            false /* fromMigrate */,
+            false /* dropSystemCollections */);
+    }
 
     ShardingLogging::get(opCtx)->logChange(opCtx, "dropCollection", nss());
     LOGV2(5390503, "Collection dropped", logAttrs(nss()));
diff --git a/src/mongo/db/s/drop_database_coordinator.cpp b/src/mongo/db/s/drop_database_coordinator.cpp
index 765c949c1f3..ae669467736 100644
--- a/src/mongo/db/s/drop_database_coordinator.cpp
+++ b/src/mongo/db/s/drop_database_coordinator.cpp
@@ -462,9 +462,11 @@ ExecutorFuture<void> DropDatabaseCoordinator::_runImpl(
                     _clearDatabaseInfoOnPrimary(opCtx);
                     _clearDatabaseInfoOnSecondaries(opCtx);
 
-                    const auto& osi = getNewSession(opCtx);
-                    removeDatabaseFromConfigAndUpdatePlacementHistory(
-                        opCtx, **executor, _dbName, *metadata().getDatabaseVersion(), osi);
+                    {
+                        const auto& session = getNewSession(opCtx);
+                        removeDatabaseFromConfigAndUpdatePlacementHistory(
+                            opCtx, **executor, _dbName, *metadata().getDatabaseVersion(), session);
+                    }
 
                     VectorClockMutable::get(opCtx)->waitForDurableConfigTime().get(opCtx);
                 }
diff --git a/src/mongo/db/s/move_primary_coordinator.cpp b/src/mongo/db/s/move_primary_coordinator.cpp
index a2a2d516977..9c8199e1b92 100644
--- a/src/mongo/db/s/move_primary_coordinator.cpp
+++ b/src/mongo/db/s/move_primary_coordinator.cpp
@@ -748,12 +748,13 @@ void MovePrimaryCoordinator::dropOrphanedDataOnRecipient(
     // Make a copy of this container since `getNewSession` changes the coordinator document.
     const auto collectionsToClone = *_doc.getCollectionsToClone();
     for (const auto& nss : collectionsToClone) {
+        const auto session = getNewSession(opCtx);
         sharding_ddl_util::sendDropCollectionParticipantCommandToShards(
             opCtx,
             nss,
             {_doc.getToShardId()},
             **executor,
-            getNewSession(opCtx),
+            session,
             true /* fromMigrate */,
             true /* dropSystemCollections */);
     }
diff --git a/src/mongo/db/s/refine_collection_shard_key_coordinator.cpp b/src/mongo/db/s/refine_collection_shard_key_coordinator.cpp
index 86f14321741..93749b34ac1 100644
--- a/src/mongo/db/s/refine_collection_shard_key_coordinator.cpp
+++ b/src/mongo/db/s/refine_collection_shard_key_coordinator.cpp
@@ -240,15 +240,18 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
                 getForwardableOpMetadata().setOn(opCtx);
 
                 if (!_firstExecution) {
-                    _performNoopWriteOnDataShardsAndConfigServer(
-                        opCtx, nss(), getNewSession(opCtx), **executor);
+                    const auto session = getNewSession(opCtx);
+                    _performNoopWriteOnDataShardsAndConfigServer(opCtx, nss(), session, **executor);
                 }
 
                 // Stop migrations before checking indexes considering any concurrent index
                 // creation/drop with migrations could leave the cluster with inconsistent indexes,
                 // PM-2077 should address that.
-                sharding_ddl_util::stopMigrations(
-                    opCtx, nss(), _request.getCollectionUUID(), getNewSession(opCtx));
+                {
+                    const auto session = getNewSession(opCtx);
+                    sharding_ddl_util::stopMigrations(
+                        opCtx, nss(), _request.getCollectionUUID(), session);
+                }
 
                 const auto& ns = nss();
                 auto catalogCache = Grid::get(opCtx)->catalogCache();
@@ -289,8 +292,8 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
                 getForwardableOpMetadata().setOn(opCtx);
 
                 if (!_firstExecution) {
-                    _performNoopWriteOnDataShardsAndConfigServer(
-                        opCtx, nss(), getNewSession(opCtx), **executor);
+                    const auto session = getNewSession(opCtx);
+                    _performNoopWriteOnDataShardsAndConfigServer(opCtx, nss(), session, **executor);
                 }
 
                 ShardsvrParticipantBlock blockCRUDOperationsRequest(nss());
@@ -331,8 +334,8 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
                 getForwardableOpMetadata().setOn(opCtx);
 
                 if (!_firstExecution) {
-                    _performNoopWriteOnDataShardsAndConfigServer(
-                        opCtx, nss(), getNewSession(opCtx), **executor);
+                    const auto session = getNewSession(opCtx);
+                    _performNoopWriteOnDataShardsAndConfigServer(opCtx, nss(), session, **executor);
                 }
 
                 ConfigsvrCommitRefineCollectionShardKey commitRequest(nss());
@@ -370,8 +373,8 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
                 getForwardableOpMetadata().setOn(opCtx);
 
                 if (!_firstExecution) {
-                    _performNoopWriteOnDataShardsAndConfigServer(
-                        opCtx, nss(), getNewSession(opCtx), **executor);
+                    const auto session = getNewSession(opCtx);
+                    _performNoopWriteOnDataShardsAndConfigServer(opCtx, nss(), session, **executor);
                 }
 
                 ShardsvrParticipantBlock unblockCRUDOperationsRequest(nss());
@@ -396,8 +399,10 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
 
                 notifyChangeStreamsOnRefineCollectionShardKeyComplete(
                     opCtx, nss(), _doc.getNewShardKey(), _doc.getOldKey().get(), *_doc.getUuid());
-                sharding_ddl_util::resumeMigrations(
-                    opCtx, nss(), boost::none, getNewSession(opCtx));
+                {
+                    const auto session = getNewSession(opCtx);
+                    sharding_ddl_util::resumeMigrations(opCtx, nss(), boost::none, session);
+                }
 
                 logRefineCollectionShardKey(opCtx, nss(), "end", BSONObj());
             }))
@@ -428,8 +433,8 @@ ExecutorFuture<void> RefineCollectionShardKeyCoordinator::_runImpl(
             // phase.
             if (!finalStatus.isOK() && _doc.getPhase() >= Phase::kRemoteIndexValidation &&
                 !_mustAlwaysMakeProgress() && !_isRetriableErrorForDDLCoordinator(finalStatus)) {
-                sharding_ddl_util::resumeMigrations(
-                    opCtx, nss(), boost::none, getNewSession(opCtx));
+                const auto session = getNewSession(opCtx);
+                sharding_ddl_util::resumeMigrations(opCtx, nss(), boost::none, session);
             }
 
             return finalStatus;
diff --git a/src/mongo/db/s/rename_collection_coordinator.cpp b/src/mongo/db/s/rename_collection_coordinator.cpp
index b9cccfb4fd3..083a528013d 100644
--- a/src/mongo/db/s/rename_collection_coordinator.cpp
+++ b/src/mongo/db/s/rename_collection_coordinator.cpp
@@ -1033,16 +1033,17 @@ ExecutorFuture<void> RenameCollectionCoordinator::_runImpl(
 
                 // Block migrations on involved collections.
                 try {
-                    const auto& osi = getNewSession(opCtx);
-                    sharding_ddl_util::stopMigrations(opCtx, fromNss, _doc.getSourceUUID(), osi);
+                    const auto session = getNewSession(opCtx);
+                    sharding_ddl_util::stopMigrations(
+                        opCtx, fromNss, _doc.getSourceUUID(), session);
                 } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>&) {
                     // stopMigrations is allowed to fail when the source collection is not tracked
                     // by the sharding catalog.
                 }
 
                 try {
-                    const auto& osi = getNewSession(opCtx);
-                    sharding_ddl_util::stopMigrations(opCtx, toNss, _doc.getTargetUUID(), osi);
+                    const auto session = getNewSession(opCtx);
+                    sharding_ddl_util::stopMigrations(opCtx, toNss, _doc.getTargetUUID(), session);
                 } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>&) {
                     // stopMigrations is allowed to fail when the target collection doesn't exist or
                     // is not tracked by the sharding catalog.
@@ -1056,8 +1057,8 @@ ExecutorFuture<void> RenameCollectionCoordinator::_runImpl(
                 getForwardableOpMetadata().setOn(opCtx);
 
                 if (!_firstExecution) {
-                    _performNoopRetryableWriteOnAllShardsAndConfigsvr(
-                        opCtx, getNewSession(opCtx), **executor);
+                    const auto session = getNewSession(opCtx);
+                    _performNoopRetryableWriteOnAllShardsAndConfigsvr(opCtx, session, **executor);
                 }
 
                 _updateNewOptTrackedCollInfoFieldAfterBinaryUpgrade();
@@ -1126,15 +1127,15 @@ ExecutorFuture<void> RenameCollectionCoordinator::_runImpl(
                 }
 
                 {
-                    const auto& osi = getNewSession(opCtx);
+                    const auto session = getNewSession(opCtx);
                     renameIndexMetadataInShards(
-                        opCtx, nss(), _request, osi, **executor, &_doc, token);
+                        opCtx, nss(), _request, session, **executor, &_doc, token);
                 }
 
                 // Update the collection metadata after the rename.
                 // Renaming the metadata will also resume migrations for the resulting collection.
                 {
-                    const auto& osi = getNewSession(opCtx);
+                    const auto session = getNewSession(opCtx);
                     renameCollectionMetadataInTransaction(
                         opCtx,
                         _doc.getOptTrackedCollInfo(),
@@ -1144,7 +1145,7 @@ ExecutorFuture<void> RenameCollectionCoordinator::_runImpl(
                         _doc.getNewTargetCollectionUuid(),
                         ShardingCatalogClient::kMajorityWriteConcern,
                         **executor,
-                        osi);
+                        session);
                 }
 
                 // Checkpoint the configTime to ensure that, in the case of a stepdown, the new
diff --git a/src/mongo/db/s/sharding_ddl_coordinator.h b/src/mongo/db/s/sharding_ddl_coordinator.h
index c61bac2072f..a96320c4c51 100644
--- a/src/mongo/db/s/sharding_ddl_coordinator.h
+++ b/src/mongo/db/s/sharding_ddl_coordinator.h
@@ -446,6 +446,8 @@ protected:
     /**
      * Advances and persists the `txnNumber` to ensure causality between requests, then returns the
      * updated operation session information (OSI).
+     * This modifies the _doc with a std::move, so any reference to members of the _doc will be
+     * invalidated after this call
      */
     OperationSessionInfo getNewSession(OperationContext* opCtx) {
         _updateSession(opCtx);
diff --git a/src/mongo/db/s/untrack_unsplittable_collection_coordinator.cpp b/src/mongo/db/s/untrack_unsplittable_collection_coordinator.cpp
index 3e8dd608d6b..d00f59d7b20 100644
--- a/src/mongo/db/s/untrack_unsplittable_collection_coordinator.cpp
+++ b/src/mongo/db/s/untrack_unsplittable_collection_coordinator.cpp
@@ -127,15 +127,18 @@ void UntrackUnsplittableCollectionCoordinator::_commitUntrackCollection(
     // guarantee targeting the config server
     const bool useClusterTransaction{true};
 
-    sharding_ddl_util::removeCollAndChunksMetadataFromConfig(
-        opCtx,
-        Grid::get(opCtx)->shardRegistry()->getConfigShard(),
-        Grid::get(opCtx)->catalogClient(),
-        _doc.getOptCollType().get(),
-        ShardingCatalogClient::kMajorityWriteConcern,
-        getNewSession(opCtx),
-        useClusterTransaction,
-        **executor);
+    {
+        const auto session = getNewSession(opCtx);
+        sharding_ddl_util::removeCollAndChunksMetadataFromConfig(
+            opCtx,
+            Grid::get(opCtx)->shardRegistry()->getConfigShard(),
+            Grid::get(opCtx)->catalogClient(),
+            _doc.getOptCollType().get(),
+            ShardingCatalogClient::kMajorityWriteConcern,
+            session,
+            useClusterTransaction,
+            **executor);
+    }
 
     // Checkpoint the configTime to ensure that, in the case of a stepdown, the new primary will
     // start-up from a configTime that is inclusive of the metadata deletions that were committed