diff options
author | Charlie Swanson <charlie.swanson@mongodb.com> | 2017-04-05 11:35:23 -0400 |
---|---|---|
committer | Charlie Swanson <charlie.swanson@mongodb.com> | 2017-04-13 16:15:20 -0400 |
commit | cc954e9e1d88b30d1ab89ee3bbbd9db0bb15263d (patch) | |
tree | 37df000f0d37d17bc82d5d1ad5436b4911249e4b /src/mongo/dbtests/query_stage_sort.cpp | |
parent | b02b7f7bb78d4fd0bb006591769faaa216e6f8a7 (diff) | |
download | mongo-cc954e9e1d88b30d1ab89ee3bbbd9db0bb15263d.tar.gz |
SERVER-25694 Eliminate race in PlanExecutor cleanup.
Ensures that a collection lock is held in at least MODE_IS while
deregistering a PlanExecutor from the cursor manager. Introduces new
PlanExecutor::dispose() and ClientCursor::dispose() methods that must be
called before destruction of those classes, and ensures they are called
before destruction. These calls will thread an OperationContext all the
way through to DocumentSource::dispose() for each stage in a Pipeline,
which will give DocumentSourceCursor a chance to acquire locks and
deregister its PlanExecutor if necessary.
Diffstat (limited to 'src/mongo/dbtests/query_stage_sort.cpp')
-rw-r--r-- | src/mongo/dbtests/query_stage_sort.cpp | 17 |
1 files changed, 9 insertions, 8 deletions
diff --git a/src/mongo/dbtests/query_stage_sort.cpp b/src/mongo/dbtests/query_stage_sort.cpp index ea5d4fb87f3..40f03c7a8c4 100644 --- a/src/mongo/dbtests/query_stage_sort.cpp +++ b/src/mongo/dbtests/query_stage_sort.cpp @@ -107,7 +107,8 @@ public: * Wraps a sort stage with a QueuedDataStage in a plan executor. Returns the plan executor, * which is owned by the caller. */ - PlanExecutor* makePlanExecutorWithSortStage(Collection* coll) { + unique_ptr<PlanExecutor, PlanExecutor::Deleter> makePlanExecutorWithSortStage( + Collection* coll) { // Build the mock scan stage which feeds the data. auto ws = make_unique<WorkingSet>(); auto queuedDataStage = make_unique<QueuedDataStage>(&_opCtx, ws.get()); @@ -128,7 +129,7 @@ public: auto statusWithPlanExecutor = PlanExecutor::make( &_opCtx, std::move(ws), std::move(ss), coll, PlanExecutor::YIELD_AUTO); invariant(statusWithPlanExecutor.isOK()); - return statusWithPlanExecutor.getValue().release(); + return std::move(statusWithPlanExecutor.getValue()); } // Return a value in the set {-1, 0, 1} to represent the sign of parameter i. Used to @@ -167,9 +168,9 @@ public: // Must fetch so we can look at the doc as a BSONObj. auto statusWithPlanExecutor = PlanExecutor::make( - &_opCtx, std::move(ws), std::move(fetchStage), coll, PlanExecutor::YIELD_MANUAL); + &_opCtx, std::move(ws), std::move(fetchStage), coll, PlanExecutor::NO_YIELD); ASSERT_OK(statusWithPlanExecutor.getStatus()); - unique_ptr<PlanExecutor> exec = std::move(statusWithPlanExecutor.getValue()); + auto exec = std::move(statusWithPlanExecutor.getValue()); // Look at pairs of objects to make sure that the sort order is pairwise (and therefore // totally) correct. @@ -328,7 +329,7 @@ public: set<RecordId> recordIds; getRecordIds(&recordIds, coll); - unique_ptr<PlanExecutor> exec(makePlanExecutorWithSortStage(coll)); + auto exec = makePlanExecutorWithSortStage(coll); SortStage* ss = static_cast<SortStage*>(exec->getRootStage()); SortKeyGeneratorStage* keyGenStage = static_cast<SortKeyGeneratorStage*>(ss->getChildren()[0].get()); @@ -437,7 +438,7 @@ public: set<RecordId> recordIds; getRecordIds(&recordIds, coll); - unique_ptr<PlanExecutor> exec(makePlanExecutorWithSortStage(coll)); + auto exec = makePlanExecutorWithSortStage(coll); SortStage* ss = static_cast<SortStage*>(exec->getRootStage()); SortKeyGeneratorStage* keyGenStage = static_cast<SortKeyGeneratorStage*>(ss->getChildren()[0].get()); @@ -566,8 +567,8 @@ public: // We don't get results back since we're sorting some parallel arrays. auto statusWithPlanExecutor = PlanExecutor::make( - &_opCtx, std::move(ws), std::move(fetchStage), coll, PlanExecutor::YIELD_MANUAL); - unique_ptr<PlanExecutor> exec = std::move(statusWithPlanExecutor.getValue()); + &_opCtx, std::move(ws), std::move(fetchStage), coll, PlanExecutor::NO_YIELD); + auto exec = std::move(statusWithPlanExecutor.getValue()); PlanExecutor::ExecState runnerState = exec->getNext(NULL, NULL); ASSERT_EQUALS(PlanExecutor::FAILURE, runnerState); |