From 79ccf499d8fa007100dd63efdca75fae28a59d98 Mon Sep 17 00:00:00 2001 From: wolfee Date: Mon, 9 Mar 2026 18:20:39 +0100 Subject: [PATCH] SERVER-120968 Mark authorization/authentication commands as non-deprioritizable (#49165) GitOrigin-RevId: 62a7a47c99e82ee0ed72881d8e849a8f607015ad --- .../execution_admission_context.cpp | 45 +++++++++++++++---- .../execution_admission_context.h | 6 ++- .../db/auth/authorization_backend_local.cpp | 11 +++++ 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/src/mongo/db/admission/execution_control/execution_admission_context.cpp b/src/mongo/db/admission/execution_control/execution_admission_context.cpp index 81023f67ba6..c4fcb5f0671 100644 --- a/src/mongo/db/admission/execution_control/execution_admission_context.cpp +++ b/src/mongo/db/admission/execution_control/execution_admission_context.cpp @@ -228,23 +228,50 @@ bool ExecutionAdmissionContext::_shouldRecordStats() { } admission::execution_control::ScopedTaskTypeModifierBase::ScopedTaskTypeModifierBase( - OperationContext* opCtx, ExecutionAdmissionContext::TaskType newValue) + OperationContext* opCtx, ExecutionAdmissionContext::TaskType newType) : _opCtx(opCtx) { - dassert(ExecutionAdmissionContext::get(_opCtx).getTaskType() == - ExecutionAdmissionContext::TaskType::Default); - dassert(ExecutionAdmissionContext::get(_opCtx).getPriority() == - AdmissionContext::Priority::kNormal); + const auto currentType = ExecutionAdmissionContext::get(_opCtx).getTaskType(); + const auto currentPrio = ExecutionAdmissionContext::get(_opCtx).getPriority(); + + dassert(currentType == newType || currentType == ExecutionAdmissionContext::TaskType::Default); + dassert(currentPrio == AdmissionContext::Priority::kExempt || + currentPrio == AdmissionContext::Priority::kNormal); dassert(!_opCtx->inMultiDocumentTransaction()); - ExecutionAdmissionContext::get(_opCtx)._taskType.store(newValue); + + ExecutionAdmissionContext::get(_opCtx)._taskType.store(newType); + + // There is no need for mutex here, because the thread <-> operation context (and + // ExecutionAdmissionContext) is a 1 to 1 relation. + // Nobody should update the ExecutionAdmissionContext's counter from a different thread (sharing + // opCtx is prohibited). + const auto recursionCount = + ++ExecutionAdmissionContext::get(_opCtx)._scopedTaskTypeModifierRecursion; + if (recursionCount < 1) { + LOGV2_WARNING(12096800, + "Inconsistency in _scopedTaskTypeModifierRecursion count. Resetting it to 1.", + "recursionCount"_attr = recursionCount); + dassert(false); + ExecutionAdmissionContext::get(_opCtx)._scopedTaskTypeModifierRecursion = 1; + } + LOGV2_DEBUG(12043501, 1, "Changing task type on ExecutionAdmissionContext", - "newValue"_attr = to_string(newValue)); + "newValue"_attr = to_string(newType)); } admission::execution_control::ScopedTaskTypeModifierBase::~ScopedTaskTypeModifierBase() { - ExecutionAdmissionContext::get(_opCtx)._taskType.store( - ExecutionAdmissionContext::TaskType::Default); + // There is no need for mutex here, because the thread <-> operation context (and + // ExecutionAdmissionContext) is a 1 to 1 relation. + // Nobody should update the ExecutionAdmissionContext's counter from a different thread (sharing + // opCtx is prohibited). + const auto recursionCount = + --ExecutionAdmissionContext::get(_opCtx)._scopedTaskTypeModifierRecursion; + dassert(recursionCount >= 0); + if (recursionCount == 0) { + ExecutionAdmissionContext::get(_opCtx)._taskType.store( + ExecutionAdmissionContext::TaskType::Default); + } } admission::execution_control::ScopedTaskTypeBackground::ScopedTaskTypeBackground( diff --git a/src/mongo/db/admission/execution_control/execution_admission_context.h b/src/mongo/db/admission/execution_control/execution_admission_context.h index b02890e6997..143d4285a16 100644 --- a/src/mongo/db/admission/execution_control/execution_admission_context.h +++ b/src/mongo/db/admission/execution_control/execution_admission_context.h @@ -275,6 +275,10 @@ private: // True if the operation was ever in a multi-document transaction. Once set to true, stays true. Atomic _wasInMultiDocTxn{false}; + + // ScopedTaskTypeModifier recursion counter to handle interleaving task type modifier + // destructions. + int _scopedTaskTypeModifierRecursion{0}; }; inline std::string to_string(ExecutionAdmissionContext::TaskType tt) { @@ -305,7 +309,7 @@ public: protected: ScopedTaskTypeModifierBase(OperationContext* opCtx, - ExecutionAdmissionContext::TaskType newValue); + ExecutionAdmissionContext::TaskType newType); private: OperationContext* _opCtx; diff --git a/src/mongo/db/auth/authorization_backend_local.cpp b/src/mongo/db/auth/authorization_backend_local.cpp index b8bb43bae8e..b9360c9185c 100644 --- a/src/mongo/db/auth/authorization_backend_local.cpp +++ b/src/mongo/db/auth/authorization_backend_local.cpp @@ -304,6 +304,8 @@ AuthorizationBackendLocal::RolesSnapshot AuthorizationBackendLocal::_snapshotRol Status AuthorizationBackendLocal::rolesExist(OperationContext* opCtx, const std::vector& roleNames) { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); + // Perform DB queries for user-defined roles (skipping builtin roles). stdx::unordered_set unknownRoles; for (const auto& roleName : roleNames) { @@ -323,6 +325,8 @@ Status AuthorizationBackendLocal::rolesExist(OperationContext* opCtx, StatusWith AuthorizationBackendLocal::resolveRoles( OperationContext* opCtx, const std::vector& roleNames, ResolveRoleOption option) try { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); + using RoleNameSet = typename decltype(ResolvedRoleData::roles)::value_type; const bool processRoles = option.shouldMineRoles(); const bool processPrivs = option.shouldMinePrivileges(); @@ -462,6 +466,8 @@ StatusWith AuthorizationBackendLocal::getUserObject( OperationContext* opCtx, const UserRequest& userReq, const SharedUserAcquisitionStats& userAcquisitionStats) try { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); + std::vector directRoles; User user(userReq.clone()); @@ -537,6 +543,8 @@ Status AuthorizationBackendLocal::getUserDescription( const UserRequest& userReq, BSONObj* result, const SharedUserAcquisitionStats& userAcquisitionStats) try { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); + const UserName& userName = userReq.getUserName(); std::vector directRoles; BSONObjBuilder resultBuilder; @@ -635,6 +643,7 @@ Status AuthorizationBackendLocal::getRolesDescription( PrivilegeFormat showPrivileges, AuthenticationRestrictionsFormat showRestrictions, std::vector* result) { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); if (showPrivileges == PrivilegeFormat::kShowAsUserFragment) { // Shouldn't be called this way, but cope if we are. @@ -704,6 +713,8 @@ Status AuthorizationBackendLocal::getRoleDescriptionsForDB( AuthenticationRestrictionsFormat showRestrictions, bool showBuiltinRoles, std::vector* result) { + admission::execution_control::ScopedTaskTypeNonDeprioritizable deprioGuard(opCtx); + auto option = makeResolveRoleOption(showPrivileges, showRestrictions); if (showPrivileges == PrivilegeFormat::kShowAsUserFragment) {