SERVER-31305 Use smart pointer to prevent memory leak in

QueryPlanner::plan()

3 files changed, 17 insertions, 18 deletions

diff --git a/src/mongo/db/query/plan_enumerator.cpp b/src/mongo/db/query/plan_enumerator.cpp
index 131e8df3363..1c8c160a9d9 100644
--- a/src/mongo/db/query/plan_enumerator.cpp
+++ b/src/mongo/db/query/plan_enumerator.cpp
@@ -340,21 +340,21 @@ PlanEnumerator::MemoID PlanEnumerator::memoIDForNode(MatchExpression* node) {
     return it->second;
 }
 
-bool PlanEnumerator::getNext(MatchExpression** tree) {
+unique_ptr<MatchExpression> PlanEnumerator::getNext() {
     if (_done) {
-        return false;
+        return nullptr;
     }
 
     // Tag with our first solution.
     tagMemo(memoIDForNode(_root));
 
-    *tree = _root->shallowClone().release();
-    tagForSort(*tree);
+    unique_ptr<MatchExpression> tree(_root->shallowClone());
+    tagForSort(tree.get());
 
     _root->resetTag();
     LOG(5) << "Enumerator: memo just before moving:" << endl << dumpMemo();
     _done = nextMemo(memoIDForNode(_root));
-    return true;
+    return tree;
 }
 
 //
diff --git a/src/mongo/db/query/plan_enumerator.h b/src/mongo/db/query/plan_enumerator.h
index dbb8403901a..91522d27488 100644
--- a/src/mongo/db/query/plan_enumerator.h
+++ b/src/mongo/db/query/plan_enumerator.h
@@ -93,12 +93,11 @@ public:
     Status init();
 
     /**
-     * Outputs a possible plan.  Leaves in the plan are tagged with an index to use.
-     * Returns true if a plan was outputted, false if no more plans will be outputted.
-     *
-     * 'tree' is set to point to the query tree.  A QueryAssignment is built from this tree.
-     * Caller owns the pointer.  Note that 'tree' itself points into data owned by the
-     * provided CanonicalQuery.
+     * Outputs a possible plan. Leaves in the plan are tagged with an index to use.
+     * Returns a MatchExpression representing a point in the query tree (which can be
+     * used to build a QueryAssignment) or nullptr if no more plans will be outputted.
+     * While owned by the caller, the MatchExpression returned points into data that is
+     * owned elsewhere.
      *
      * Nodes in 'tree' are tagged with indices that should be used to answer the tagged nodes.
      * Only nodes that have a field name (getCategory() != kLogical) will be tagged.
@@ -106,7 +105,7 @@ public:
      * The output tree is a clone identical to that used to initialize the enumerator, with tags
      * added in order to indicate index usage.
      */
-    bool getNext(MatchExpression** tree);
+    std::unique_ptr<MatchExpression> getNext();
 
 private:
     //
diff --git a/src/mongo/db/query/query_planner.cpp b/src/mongo/db/query/query_planner.cpp
index a184606e256..2f298269e31 100644
--- a/src/mongo/db/query/query_planner.cpp
+++ b/src/mongo/db/query/query_planner.cpp
@@ -865,15 +865,15 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
         PlanEnumerator isp(enumParams);
         isp.init().transitional_ignore();
 
-        MatchExpression* rawTree;
-        while (isp.getNext(&rawTree) && (out->size() < params.maxIndexedSolutions)) {
+        unique_ptr<MatchExpression> rawTree;
+        while ((rawTree = isp.getNext()) && (out->size() < params.maxIndexedSolutions)) {
             LOG(5) << "About to build solntree from tagged tree:" << endl
-                   << redact(rawTree->toString());
+                   << redact(rawTree.get()->toString());
 
             // Store the plan cache index tree before calling prepareForAccessingPlanning(), so that
             // the PlanCacheIndexTree has the same sort as the MatchExpression used to generate the
             // plan cache key.
-            std::unique_ptr<MatchExpression> clone(rawTree->shallowClone());
+            std::unique_ptr<MatchExpression> clone(rawTree.get()->shallowClone());
             PlanCacheIndexTree* cacheData;
             Status indexTreeStatus =
                 cacheDataFromTaggedTree(clone.get(), relevantIndices, &cacheData);
@@ -884,11 +884,11 @@ Status QueryPlanner::plan(const CanonicalQuery& query,
 
             // We have already cached the tree in canonical order, so now we can order the nodes for
             // access planning.
-            prepareForAccessPlanning(rawTree);
+            prepareForAccessPlanning(rawTree.get());
 
             // This can fail if enumeration makes a mistake.
             std::unique_ptr<QuerySolutionNode> solnRoot(QueryPlannerAccess::buildIndexedDataAccess(
-                query, rawTree, false, relevantIndices, params));
+                query, rawTree.release(), false, relevantIndices, params));
 
             if (!solnRoot) {
                 continue;