diff options
author | George Wangensteen <george.wangensteen@mongodb.com> | 2022-09-16 14:30:49 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2022-09-16 14:58:48 +0000 |
commit | 228a21b45bbe9978c80059538d2136afd337fed8 (patch) | |
tree | 6be1480b7d0f789f3bbdf514064d313dc08fdedb | |
parent | 8fefbc6c5545b188900d26c0d4d448628a9ba98b (diff) | |
download | mongo-228a21b45bbe9978c80059538d2136afd337fed8.tar.gz |
SERVER-69702 Ensure TaskExecutorCursor's constructed from multi-cursor replies have correct OperationContext
-rw-r--r-- | src/mongo/executor/task_executor_cursor.cpp | 4 | ||||
-rw-r--r-- | src/mongo/executor/task_executor_cursor_test.cpp | 44 |
2 files changed, 47 insertions, 1 deletions
diff --git a/src/mongo/executor/task_executor_cursor.cpp b/src/mongo/executor/task_executor_cursor.cpp index 694a1bc9e97..015fb9944dd 100644 --- a/src/mongo/executor/task_executor_cursor.cpp +++ b/src/mongo/executor/task_executor_cursor.cpp @@ -245,10 +245,12 @@ void TaskExecutorCursor::_getNextBatch(OperationContext* opCtx) { _processResponse(opCtx, std::move(cr)); // If we have more responses, build them into cursors then hold them until a caller accesses // them. Skip the first response, we used it to populate this cursor. + // Ensure we update the RCR we give to each 'child cursor' with the current opCtx. + auto freshRcr = _createRequest(opCtx, _rcr.cmdObj); for (unsigned int i = 1; i < cursorResponses.size(); ++i) { _additionalCursors.emplace_back(_executor, uassertStatusOK(std::move(cursorResponses[i])), - _rcr, + freshRcr, TaskExecutorCursor::Options()); } } diff --git a/src/mongo/executor/task_executor_cursor_test.cpp b/src/mongo/executor/task_executor_cursor_test.cpp index 1528f36a633..966603a517f 100644 --- a/src/mongo/executor/task_executor_cursor_test.cpp +++ b/src/mongo/executor/task_executor_cursor_test.cpp @@ -211,6 +211,50 @@ TEST_F(TaskExecutorCursorFixture, MultipleCursorsSingleBatchSucceeds) { ASSERT_FALSE(secondCursor.getNext(opCtx.get())); } +/** + * The operation context under which we send the original cursor-establishing command + * can be destructed before getNext is called with new opCtx. Ensure that 'child' + * TaskExecutorCursors created from the original TEC's multi-cursor-response can safely + * operate if this happens/don't try and use the now-destroyed operation context. + * See SERVER-69702 for context + */ +TEST_F(TaskExecutorCursorFixture, ChildTaskExecutorCursorsAreSafeIfOriginalOpCtxDestructed) { + auto lsid = makeLogicalSessionIdForTest(); + opCtx->setLogicalSessionId(lsid); + const auto aggCmd = BSON("aggregate" + << "test" + << "pipeline" << BSON_ARRAY(BSON("returnMultipleCursors" << true))); + RemoteCommandRequest rcr(HostAndPort("localhost"), "test", aggCmd, opCtx.get()); + TaskExecutorCursor tec(&getExecutor(), rcr); + auto expected = BSON("aggregate" + << "test" + << "pipeline" << BSON_ARRAY(BSON("returnMultipleCursors" << true)) + << "lsid" << lsid.toBSON()); + ASSERT_BSONOBJ_EQ(expected, scheduleSuccessfulMultiCursorResponse("firstBatch", 1, 2, {0, 0})); + // Before calling getNext (and therefore spawning child TECs), destroy the opCtx + // we used to send the initial query and make a new one. + opCtx.reset(); + opCtx = client->makeOperationContext(); + opCtx->setLogicalSessionId(lsid); + // Use the new opCtx to call getNext. The child TECs should not attempt to read from the + // now dead original opCtx. + ASSERT_EQUALS(tec.getNext(opCtx.get()).value()["x"].Int(), 1); + + ASSERT_EQUALS(tec.getNext(opCtx.get()).value()["x"].Int(), 2); + + ASSERT_FALSE(tec.getNext(opCtx.get())); + + auto cursorVec = tec.releaseAdditionalCursors(); + ASSERT_EQUALS(cursorVec.size(), 1); + auto secondCursor = std::move(cursorVec[0]); + + ASSERT_EQUALS(secondCursor.getNext(opCtx.get()).value()["x"].Int(), 2); + ASSERT_EQUALS(secondCursor.getNext(opCtx.get()).value()["x"].Int(), 4); + ASSERT_FALSE(hasReadyRequests()); + + ASSERT_FALSE(secondCursor.getNext(opCtx.get())); +} + TEST_F(TaskExecutorCursorFixture, MultipleCursorsGetMoreWorks) { const auto aggCmd = BSON("aggregate" << "test" |