diff options
author | Jennifer Peshansky <jennifer.peshansky@mongodb.com> | 2021-07-16 20:53:20 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-07-27 00:01:51 +0000 |
commit | 81dd7b36b863e57c0cbba1032a550bb55f353390 (patch) | |
tree | 80ffe8fe4aeb2f2c3e1c70fee9ebe013badc2dca /src/mongo | |
parent | 104a14d151133302184522921a1a3225c8eb8824 (diff) | |
download | mongo-81dd7b36b863e57c0cbba1032a550bb55f353390.tar.gz |
SERVER-58403 Serialize concurrent accesses to OperationContext::_comment
Diffstat (limited to 'src/mongo')
-rw-r--r-- | src/mongo/db/commands/explain_cmd.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/commands/getmore_cmd.cpp | 1 | ||||
-rw-r--r-- | src/mongo/db/curop_test.cpp | 5 | ||||
-rw-r--r-- | src/mongo/db/operation_context.h | 7 | ||||
-rw-r--r-- | src/mongo/db/operation_context_test.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/s/forwardable_operation_metadata.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/service_entry_point_common.cpp | 1 | ||||
-rw-r--r-- | src/mongo/s/commands/cluster_explain_cmd.cpp | 1 | ||||
-rw-r--r-- | src/mongo/s/commands/strategy.cpp | 5 | ||||
-rw-r--r-- | src/mongo/s/query/cluster_find.cpp | 2 |
10 files changed, 26 insertions, 5 deletions
diff --git a/src/mongo/db/commands/explain_cmd.cpp b/src/mongo/db/commands/explain_cmd.cpp index 318029aef75..34f3965d454 100644 --- a/src/mongo/db/commands/explain_cmd.cpp +++ b/src/mongo/db/commands/explain_cmd.cpp @@ -165,6 +165,7 @@ std::unique_ptr<CommandInvocation> CmdExplain::parse(OperationContext* opCtx, // Extract 'comment' field from the 'explainedObj' only if there is no top-level comment. auto commentField = explainedObj["comment"]; if (!opCtx->getComment() && commentField) { + stdx::lock_guard<Client> lk(*opCtx->getClient()); opCtx->setComment(commentField.wrap()); } diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp index 2652cfbc7e9..901f62ef6d7 100644 --- a/src/mongo/db/commands/getmore_cmd.cpp +++ b/src/mongo/db/commands/getmore_cmd.cpp @@ -239,6 +239,7 @@ void setUpOperationContextStateForGetMore(OperationContext* opCtx, // that if the 'getMore' command itself has a 'comment' field, we give precedence to it. auto comment = cursor.getOriginatingCommandObj()["comment"]; if (!opCtx->getComment() && comment) { + stdx::lock_guard<Client> lk(*opCtx->getClient()); opCtx->setComment(comment.wrap()); } } diff --git a/src/mongo/db/curop_test.cpp b/src/mongo/db/curop_test.cpp index 857ef674d52..332e7d9042e 100644 --- a/src/mongo/db/curop_test.cpp +++ b/src/mongo/db/curop_test.cpp @@ -232,7 +232,10 @@ TEST(CurOpTest, ShouldNotReportFailpointMsgIfNotSet) { // Test the reported state should _not_ contain 'failpointMsg'. BSONObjBuilder reportedStateWithoutFailpointMsg; - curop->reportState(opCtx.get(), &reportedStateWithoutFailpointMsg); + { + stdx::lock_guard<Client> lk(*opCtx->getClient()); + curop->reportState(opCtx.get(), &reportedStateWithoutFailpointMsg); + } auto bsonObj = reportedStateWithoutFailpointMsg.done(); // bsonObj should _not_ contain 'failpointMsg' if a fail point is not set. diff --git a/src/mongo/db/operation_context.h b/src/mongo/db/operation_context.h index bbb89ecb739..010ab14756c 100644 --- a/src/mongo/db/operation_context.h +++ b/src/mongo/db/operation_context.h @@ -518,10 +518,17 @@ public: _isStartingMultiDocumentTransaction = isStartingMultiDocumentTransaction; } + /** + * Sets '_comment'. The client lock must be acquired before calling this method. + */ void setComment(const BSONObj& comment) { _comment = comment.getOwned(); } + /** + * Gets '_comment'. The client lock must be acquired when calling from any thread that does + * not own the client associated with the operation. + */ boost::optional<BSONElement> getComment() { // The '_comment' object, if present, will only ever have one field. return _comment ? boost::optional<BSONElement>(_comment->firstElement()) : boost::none; diff --git a/src/mongo/db/operation_context_test.cpp b/src/mongo/db/operation_context_test.cpp index e06c084101a..66fcda8632a 100644 --- a/src/mongo/db/operation_context_test.cpp +++ b/src/mongo/db/operation_context_test.cpp @@ -1151,6 +1151,8 @@ TEST(OperationContextTest, CurrentOpExcludesKilledOperations) { // of an `opCtx`. This is because `CurOp::reportCurrentOpForClient()` accepts an `opCtx` // as input and requires it to be present throughout its execution. stdx::thread thread([&]() mutable { + stdx::lock_guard<Client> lk(*opCtx->getClient()); + auto threadClient = serviceCtx->makeClient("ThreadClient"); // Generate report in absence of any opCtx diff --git a/src/mongo/db/s/forwardable_operation_metadata.cpp b/src/mongo/db/s/forwardable_operation_metadata.cpp index 45371ee0b26..b49c73f9fc9 100644 --- a/src/mongo/db/s/forwardable_operation_metadata.cpp +++ b/src/mongo/db/s/forwardable_operation_metadata.cpp @@ -51,15 +51,17 @@ ForwardableOperationMetadata::ForwardableOperationMetadata(OperationContext* opC } void ForwardableOperationMetadata::setOn(OperationContext* opCtx) const { + Client* client = opCtx->getClient(); if (const auto& comment = getComment()) { + stdx::lock_guard<Client> lk(*client); opCtx->setComment(comment.get()); } if (const auto& optAuthMetadata = getImpersonatedUserMetadata()) { const auto& authMetadata = optAuthMetadata.get(); if (!authMetadata.getUsers().empty() || !authMetadata.getRoles().empty()) { - AuthorizationSession::get(opCtx->getClient()) - ->setImpersonatedUserData(authMetadata.getUsers(), authMetadata.getRoles()); + AuthorizationSession::get(client)->setImpersonatedUserData(authMetadata.getUsers(), + authMetadata.getRoles()); } } } diff --git a/src/mongo/db/service_entry_point_common.cpp b/src/mongo/db/service_entry_point_common.cpp index 0b7ec6d4ee3..28cca1ed035 100644 --- a/src/mongo/db/service_entry_point_common.cpp +++ b/src/mongo/db/service_entry_point_common.cpp @@ -1337,6 +1337,7 @@ void ExecCommandDatabase::_initiateCommand() { } else if (fieldName == CommandHelpers::kHelpFieldName) { helpField = element; } else if (fieldName == "comment") { + stdx::lock_guard<Client> lk(*client); opCtx->setComment(element.wrap()); } else if (fieldName == query_request_helper::queryOptionMaxTimeMS) { uasserted(ErrorCodes::InvalidOptions, diff --git a/src/mongo/s/commands/cluster_explain_cmd.cpp b/src/mongo/s/commands/cluster_explain_cmd.cpp index 9ee65bdbc49..e7734ec43fb 100644 --- a/src/mongo/s/commands/cluster_explain_cmd.cpp +++ b/src/mongo/s/commands/cluster_explain_cmd.cpp @@ -180,6 +180,7 @@ std::unique_ptr<CommandInvocation> ClusterExplainCmd::parse(OperationContext* op // Extract 'comment' field from the 'explainedObj' only if there is no top-level comment. auto commentField = explainedObj["comment"]; if (!opCtx->getComment() && commentField) { + stdx::lock_guard<Client> lk(*opCtx->getClient()); opCtx->setComment(commentField.wrap()); } diff --git a/src/mongo/s/commands/strategy.cpp b/src/mongo/s/commands/strategy.cpp index 5a80b17e933..db2d1c2ceda 100644 --- a/src/mongo/s/commands/strategy.cpp +++ b/src/mongo/s/commands/strategy.cpp @@ -500,7 +500,8 @@ void ParseAndRunCommand::_parseCommand() { _isHello.emplace(command->getName() == "hello"_sd || command->getName() == "isMaster"_sd); opCtx->setExhaust(OpMsg::isFlagSet(m, OpMsg::kExhaustSupported)); - const auto session = opCtx->getClient()->session(); + Client* client = opCtx->getClient(); + const auto session = client->session(); if (session) { if (!opCtx->isExhaust() || !_isHello.get()) { InExhaustHello::get(session.get())->setInExhaust(false, _commandName); @@ -529,10 +530,10 @@ void ParseAndRunCommand::_parseCommand() { // If the command includes a 'comment' field, set it on the current OpCtx. if (auto commentField = request.body["comment"]) { + stdx::lock_guard<Client> lk(*client); opCtx->setComment(commentField.wrap()); } - Client* client = opCtx->getClient(); auto const apiParamsFromClient = initializeAPIParameters(request.body, command); { diff --git a/src/mongo/s/query/cluster_find.cpp b/src/mongo/s/query/cluster_find.cpp index 7ed9694090c..cca6cf1ee02 100644 --- a/src/mongo/s/query/cluster_find.cpp +++ b/src/mongo/s/query/cluster_find.cpp @@ -466,8 +466,10 @@ Status setUpOperationContextStateForGetMore(OperationContext* opCtx, // that if the 'getMore' command itself has a 'comment' field, we give precedence to it. auto comment = cursor->getOriginatingCommand()["comment"]; if (!opCtx->getComment() && comment) { + stdx::lock_guard<Client> lk(*opCtx->getClient()); opCtx->setComment(comment.wrap()); } + if (cursor->isTailableAndAwaitData()) { // For tailable + awaitData cursors, the request may have indicated a maximum amount of time // to wait for new data. If not, default it to 1 second. We track the deadline instead via |