From 004183f093d99d6f54615eb3bb8caeb089ff47ac Mon Sep 17 00:00:00 2001
From: Mark Benvenuto <mark.benvenuto@mongodb.com>
Date: Fri, 4 May 2018 10:18:11 -0400
Subject: [PATCH] SERVER-34791 Free Mon Fix locking

---
 jstests/free_mon/libs/free_mon.js          |  4 +-
 src/mongo/db/free_mon/free_mon_storage.cpp | 55 +++++++++-------------
 2 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/jstests/free_mon/libs/free_mon.js b/jstests/free_mon/libs/free_mon.js
index 01471020faf..1fd3c3c01e1 100644
--- a/jstests/free_mon/libs/free_mon.js
+++ b/jstests/free_mon/libs/free_mon.js
@@ -137,7 +137,7 @@ class FreeMonWebServer {
         assert.soon(function() {
             const stats = qs();
             print("QS : " + tojson(stats));
-            return stats.registers == count;
+            return stats.registers >= count;
         }, "Failed to web server register", 60 * 1000);
     }
 
@@ -152,7 +152,7 @@ class FreeMonWebServer {
         assert.soon(function() {
             const stats = qs();
             print("QS : " + tojson(stats));
-            return stats.metrics == count;
+            return stats.metrics >= count;
         }, "Failed to web server metrics", 60 * 1000);
     }
 }
diff --git a/src/mongo/db/free_mon/free_mon_storage.cpp b/src/mongo/db/free_mon/free_mon_storage.cpp
index 978be046eba..e9abd67020a 100644
--- a/src/mongo/db/free_mon/free_mon_storage.cpp
+++ b/src/mongo/db/free_mon/free_mon_storage.cpp
@@ -32,8 +32,7 @@
 #include "mongo/base/string_data.h"
 #include "mongo/bson/bsonelement.h"
 #include "mongo/bson/bsonmisc.h"
-#include "mongo/db/concurrency/d_concurrency.h"
-#include "mongo/db/concurrency/lock_manager_defs.h"
+#include "mongo/db/db_raii.h"
 #include "mongo/db/namespace_string.h"
 #include "mongo/db/operation_context.h"
 #include "mongo/db/repl/replication_coordinator.h"
@@ -57,9 +56,7 @@ boost::optional<FreeMonStorageState> FreeMonStorage::read(OperationContext* opCt
 
     auto storageInterface = repl::StorageInterface::get(opCtx);
 
-    Lock::DBLock dblk(opCtx, NamespaceString::kServerConfigurationNamespace.db(), MODE_IS);
-    Lock::CollectionLock lk(
-        opCtx->lockState(), NamespaceString::kServerConfigurationNamespace.ns(), MODE_IS);
+    AutoGetCollectionForRead autoRead(opCtx, NamespaceString::kServerConfigurationNamespace);
 
     auto swObj = storageInterface->findById(
         opCtx, NamespaceString::kServerConfigurationNamespace, elementKey);
@@ -82,18 +79,14 @@ void FreeMonStorage::replace(OperationContext* opCtx, const FreeMonStorageState&
     BSONObj obj = doc.toBSON();
 
     auto storageInterface = repl::StorageInterface::get(opCtx);
-    {
-        Lock::DBLock dblk(opCtx, NamespaceString::kServerConfigurationNamespace.db(), MODE_IS);
-        Lock::CollectionLock lk(
-            opCtx->lockState(), NamespaceString::kServerConfigurationNamespace.ns(), MODE_IS);
+    AutoGetCollection autoWrite(opCtx, NamespaceString::kServerConfigurationNamespace, MODE_IX);
 
-        if (repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(
-                opCtx, NamespaceString::kServerConfigurationNamespace)) {
-            auto swObj = storageInterface->upsertById(
-                opCtx, NamespaceString::kServerConfigurationNamespace, elementKey, obj);
-            if (!swObj.isOK()) {
-                uassertStatusOK(swObj);
-            }
+    if (repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(
+            opCtx, NamespaceString::kServerConfigurationNamespace)) {
+        auto swObj = storageInterface->upsertById(
+            opCtx, NamespaceString::kServerConfigurationNamespace, elementKey, obj);
+        if (!swObj.isOK()) {
+            uassertStatusOK(swObj);
         }
     }
 }
@@ -103,24 +96,20 @@ void FreeMonStorage::deleteState(OperationContext* opCtx) {
     BSONElement elementKey = deleteKey.firstElement();
 
     auto storageInterface = repl::StorageInterface::get(opCtx);
-    {
-        Lock::DBLock dblk(opCtx, NamespaceString::kServerConfigurationNamespace.db(), MODE_IS);
-        Lock::CollectionLock lk(
-            opCtx->lockState(), NamespaceString::kServerConfigurationNamespace.ns(), MODE_IS);
+    AutoGetCollection autoWrite(opCtx, NamespaceString::kServerConfigurationNamespace, MODE_IX);
 
-        if (repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(
-                opCtx, NamespaceString::kServerConfigurationNamespace)) {
+    if (repl::ReplicationCoordinator::get(opCtx)->canAcceptWritesFor(
+            opCtx, NamespaceString::kServerConfigurationNamespace)) {
 
-            auto swObj = storageInterface->deleteById(
-                opCtx, NamespaceString::kServerConfigurationNamespace, elementKey);
-            if (!swObj.isOK()) {
-                // Ignore errors about no document
-                if (swObj.getStatus() == ErrorCodes::NoSuchKey) {
-                    return;
-                }
-
-                uassertStatusOK(swObj);
+        auto swObj = storageInterface->deleteById(
+            opCtx, NamespaceString::kServerConfigurationNamespace, elementKey);
+        if (!swObj.isOK()) {
+            // Ignore errors about no document
+            if (swObj.getStatus() == ErrorCodes::NoSuchKey) {
+                return;
             }
+
+            uassertStatusOK(swObj);
         }
     }
 }
@@ -128,9 +117,7 @@ void FreeMonStorage::deleteState(OperationContext* opCtx) {
 boost::optional<BSONObj> FreeMonStorage::readClusterManagerState(OperationContext* opCtx) {
     auto storageInterface = repl::StorageInterface::get(opCtx);
 
-    Lock::DBLock dblk(opCtx, NamespaceString::kServerConfigurationNamespace.db(), MODE_IS);
-    Lock::CollectionLock lk(
-        opCtx->lockState(), NamespaceString::kServerConfigurationNamespace.ns(), MODE_IS);
+    AutoGetCollectionForRead autoRead(opCtx, NamespaceString::kServerConfigurationNamespace);
 
     auto swObj = storageInterface->findSingleton(opCtx, localClusterManagerNss);
     if (!swObj.isOK()) {