SERVER-50700 Allow constructing a FailPointEnableBlock from a pointer to the FailPoint

3 files changed, 81 insertions, 49 deletions

diff --git a/src/mongo/util/fail_point.cpp b/src/mongo/util/fail_point.cpp
index c0a28ddb3ac..e467ff2d9fb 100644
--- a/src/mongo/util/fail_point.cpp
+++ b/src/mongo/util/fail_point.cpp
@@ -70,7 +70,7 @@ void FailPoint::setThreadPRNGSeed(int32_t seed) {
     threadPrng = PseudoRandom(seed);
 }
 
-FailPoint::FailPoint() = default;
+FailPoint::FailPoint(std::string name) : _name(std::move(name)) {}
 
 void FailPoint::_shouldFailCloseBlock() {
     _fpInfo.subtractAndFetch(1);
@@ -286,8 +286,8 @@ BSONObj FailPoint::toBSON() const {
     return builder.obj();
 }
 
-FailPointRegisterer::FailPointRegisterer(const std::string& name, FailPoint* fp) {
-    uassertStatusOK(globalFailPointRegistry().add(name, fp));
+FailPointRegisterer::FailPointRegisterer(FailPoint* fp) {
+    uassertStatusOK(globalFailPointRegistry().add(fp));
 }
 
 FailPointRegistry& globalFailPointRegistry() {
@@ -309,12 +309,18 @@ auto setGlobalFailPoint(const std::string& failPointName, const BSONObj& cmdObj)
     return timesEntered;
 }
 
-FailPointEnableBlock::FailPointEnableBlock(std::string failPointName)
-    : FailPointEnableBlock(std::move(failPointName), {}) {}
+FailPointEnableBlock::FailPointEnableBlock(StringData failPointName)
+    : FailPointEnableBlock(failPointName, {}) {}
+
+FailPointEnableBlock::FailPointEnableBlock(StringData failPointName, BSONObj data)
+    : FailPointEnableBlock(globalFailPointRegistry().find(failPointName), std::move(data)) {}
+
+FailPointEnableBlock::FailPointEnableBlock(FailPoint* failPoint)
+    : FailPointEnableBlock(failPoint, {}) {}
+
+FailPointEnableBlock::FailPointEnableBlock(FailPoint* failPoint, BSONObj data)
+    : _failPoint(failPoint) {
 
-FailPointEnableBlock::FailPointEnableBlock(std::string failPointName, BSONObj data)
-    : _failPointName(std::move(failPointName)) {
-    _failPoint = globalFailPointRegistry().find(_failPointName);
     invariant(_failPoint != nullptr);
 
     _initialTimesEntered = _failPoint->setMode(FailPoint::alwaysOn, 0, std::move(data));
@@ -322,7 +328,7 @@ FailPointEnableBlock::FailPointEnableBlock(std::string failPointName, BSONObj da
     LOGV2_WARNING(23830,
                   "Set failpoint {failPointName} to: {failPoint}",
                   "Set failpoint",
-                  "failPointName"_attr = _failPointName,
+                  "failPointName"_attr = _failPoint->getName(),
                   "failPoint"_attr = _failPoint->toBSON());
 }
 
@@ -331,24 +337,25 @@ FailPointEnableBlock::~FailPointEnableBlock() {
     LOGV2_WARNING(23831,
                   "Set failpoint {failPointName} to: {failPoint}",
                   "Set failpoint",
-                  "failPointName"_attr = _failPointName,
+                  "failPointName"_attr = _failPoint->getName(),
                   "failPoint"_attr = _failPoint->toBSON());
 }
 
 FailPointRegistry::FailPointRegistry() : _frozen(false) {}
 
-Status FailPointRegistry::add(const std::string& name, FailPoint* failPoint) {
+Status FailPointRegistry::add(FailPoint* failPoint) {
     if (_frozen) {
         return {ErrorCodes::CannotMutateObject, "Registry is already frozen"};
     }
-    auto [pos, ok] = _fpMap.insert({name, failPoint});
+    auto [pos, ok] = _fpMap.insert({failPoint->getName(), failPoint});
     if (!ok) {
-        return {ErrorCodes::Error(51006), "Fail point already registered: {}"_format(name)};
+        return {ErrorCodes::Error(51006),
+                "Fail point already registered: {}"_format(failPoint->getName())};
     }
     return Status::OK();
 }
 
-FailPoint* FailPointRegistry::find(const std::string& name) const {
+FailPoint* FailPointRegistry::find(StringData name) const {
     auto iter = _fpMap.find(name);
     return (iter == _fpMap.end()) ? nullptr : iter->second;
 }
diff --git a/src/mongo/util/fail_point.h b/src/mongo/util/fail_point.h
index 5322fad8b67..af02e9f1622 100644
--- a/src/mongo/util/fail_point.h
+++ b/src/mongo/util/fail_point.h
@@ -40,6 +40,7 @@
 #include "mongo/stdx/unordered_map.h"
 #include "mongo/util/duration.h"
 #include "mongo/util/interruptible.h"
+#include "mongo/util/string_map.h"
 
 namespace mongo {
 
@@ -202,11 +203,15 @@ public:
      */
     static StatusWith<ModeOptions> parseBSON(const BSONObj& obj);
 
-    FailPoint();
+    explicit FailPoint(std::string name);
 
     FailPoint(const FailPoint&) = delete;
     FailPoint& operator=(const FailPoint&) = delete;
 
+    const std::string& getName() const {
+        return _name;
+    }
+
     /**
      * Returns true if fail point is active.
      *
@@ -423,6 +428,8 @@ private:
     AtomicWord<int> _timesOrPeriod{0};
     BSONObj _data;
 
+    const std::string _name;
+
     // protects _mode, _timesOrPeriod, _data
     mutable Mutex _modMutex = MONGO_MAKE_LATCH("FailPoint::_modMutex");
 };
@@ -439,12 +446,12 @@ public:
      *     51006 - if the given name already exists in this registry.
      *     CannotMutateObject - if this registry is already frozen.
      */
-    Status add(const std::string& name, FailPoint* failPoint);
+    Status add(FailPoint* failPoint);
 
     /**
      * @return a registered FailPoint, or nullptr if it was not registered.
      */
-    FailPoint* find(const std::string& name) const;
+    FailPoint* find(StringData name) const;
 
     /**
      * Freezes this registry from being modified.
@@ -460,7 +467,7 @@ public:
 
 private:
     bool _frozen;
-    stdx::unordered_map<std::string, FailPoint*> _fpMap;
+    StringMap<FailPoint*> _fpMap;
 };
 
 /**
@@ -468,10 +475,15 @@ private:
  */
 class FailPointEnableBlock {
 public:
-    explicit FailPointEnableBlock(std::string failPointName);
-    FailPointEnableBlock(std::string failPointName, BSONObj data);
+    explicit FailPointEnableBlock(StringData failPointName);
+    FailPointEnableBlock(StringData failPointName, BSONObj data);
+    explicit FailPointEnableBlock(FailPoint* failPoint);
+    FailPointEnableBlock(FailPoint* failPoint, BSONObj data);
     ~FailPointEnableBlock();
 
+    FailPointEnableBlock(const FailPointEnableBlock&) = delete;
+    FailPointEnableBlock& operator=(const FailPointEnableBlock&) = delete;
+
     // Const access to the underlying FailPoint
     const FailPoint* failPoint() const {
         return _failPoint;
@@ -488,8 +500,7 @@ public:
     }
 
 private:
-    std::string _failPointName;
-    FailPoint* _failPoint;
+    FailPoint* const _failPoint;
     FailPoint::EntryCountT _initialTimesEntered;
 };
 
@@ -507,7 +518,7 @@ FailPoint::EntryCountT setGlobalFailPoint(const std::string& failPointName, cons
  */
 class FailPointRegisterer {
 public:
-    FailPointRegisterer(const std::string& name, FailPoint* fp);
+    explicit FailPointRegisterer(FailPoint* fp);
 };
 
 FailPointRegistry& globalFailPointRegistry();
@@ -518,8 +529,8 @@ FailPointRegistry& globalFailPointRegistry();
  * Never use in header files, only .cpp files.
  */
 #define MONGO_FAIL_POINT_DEFINE(fp) \
-    ::mongo::FailPoint fp;          \
-    ::mongo::FailPointRegisterer fp##failPointRegisterer(#fp, &fp);
+    ::mongo::FailPoint fp(#fp);     \
+    ::mongo::FailPointRegisterer fp##failPointRegisterer(&fp);
 
 
 }  // namespace mongo
diff --git a/src/mongo/util/fail_point_test.cpp b/src/mongo/util/fail_point_test.cpp
index 26b051fb7dc..a3c346594c9 100644
--- a/src/mongo/util/fail_point_test.cpp
+++ b/src/mongo/util/fail_point_test.cpp
@@ -55,12 +55,12 @@ namespace stdx = mongo::stdx;
 
 namespace mongo_test {
 TEST(FailPoint, InitialState) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     ASSERT_FALSE(failPoint.shouldFail());
 }
 
 TEST(FailPoint, AlwaysOn) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn);
     ASSERT(failPoint.shouldFail());
 
@@ -74,7 +74,7 @@ TEST(FailPoint, AlwaysOn) {
 }
 
 TEST(FailPoint, NTimes) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::nTimes, 4);
     ASSERT(failPoint.shouldFail());
     ASSERT(failPoint.shouldFail());
@@ -87,14 +87,14 @@ TEST(FailPoint, NTimes) {
 }
 
 TEST(FailPoint, BlockOff) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     bool called = false;
     failPoint.execute([&](const BSONObj&) { called = true; });
     ASSERT_FALSE(called);
 }
 
 TEST(FailPoint, BlockAlwaysOn) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn);
     bool called = false;
 
@@ -104,7 +104,7 @@ TEST(FailPoint, BlockAlwaysOn) {
 }
 
 TEST(FailPoint, BlockNTimes) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::nTimes, 1);
     size_t counter = 0;
 
@@ -116,7 +116,7 @@ TEST(FailPoint, BlockNTimes) {
 }
 
 TEST(FailPoint, BlockWithException) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn);
     bool threw = false;
 
@@ -134,7 +134,7 @@ TEST(FailPoint, BlockWithException) {
 }
 
 TEST(FailPoint, SetGetParam) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn, 0, BSON("x" << 20));
 
     failPoint.execute([&](const BSONObj& data) { ASSERT_EQUALS(20, data["x"].numberInt()); });
@@ -143,12 +143,13 @@ TEST(FailPoint, SetGetParam) {
 class FailPointStress : public mongo::unittest::Test {
 public:
     void setUp() {
-        _fp.setMode(FailPoint::alwaysOn, 0, BSON("a" << 44));
+        _fp = std::make_unique<FailPoint>("testFP");
+        _fp->setMode(FailPoint::alwaysOn, 0, BSON("a" << 44));
     }
 
     void tearDown() {
         // Note: This can loop indefinitely if reference counter was off
-        _fp.setMode(FailPoint::off, 0, BSON("a" << 66));
+        _fp->setMode(FailPoint::off, 0, BSON("a" << 66));
     }
 
     void startTest() {
@@ -174,7 +175,7 @@ public:
 private:
     void blockTask() {
         while (true) {
-            _fp.execute([](const BSONObj& data) {
+            _fp->execute([](const BSONObj& data) {
                 // Expanded ASSERT_EQUALS since the error is not being
                 // printed out properly
                 if (data["a"].numberInt() != 44) {
@@ -196,7 +197,7 @@ private:
     void blockWithExceptionTask() {
         while (true) {
             try {
-                _fp.execute([](const BSONObj& data) {
+                _fp->execute([](const BSONObj& data) {
                     if (data["a"].numberInt() != 44) {
                         using namespace mongo::literals;
                         LOGV2_ERROR(24130,
@@ -219,7 +220,7 @@ private:
 
     void simpleTask() {
         while (true) {
-            static_cast<void>(MONGO_unlikely(_fp.shouldFail()));
+            static_cast<void>(MONGO_unlikely(_fp->shouldFail()));
             stdx::lock_guard<mongo::Latch> lk(_mutex);
             if (_inShutdown)
                 break;
@@ -228,10 +229,10 @@ private:
 
     void flipTask() {
         while (true) {
-            if (_fp.shouldFail()) {
-                _fp.setMode(FailPoint::off, 0);
+            if (_fp->shouldFail()) {
+                _fp->setMode(FailPoint::off, 0);
             } else {
-                _fp.setMode(FailPoint::alwaysOn, 0, BSON("a" << 44));
+                _fp->setMode(FailPoint::alwaysOn, 0, BSON("a" << 44));
             }
 
             stdx::lock_guard<mongo::Latch> lk(_mutex);
@@ -240,7 +241,7 @@ private:
         }
     }
 
-    FailPoint _fp;
+    std::unique_ptr<FailPoint> _fp;
     std::vector<stdx::thread> _tasks;
 
     mongo::Mutex _mutex = MONGO_MAKE_LATCH();
@@ -249,7 +250,7 @@ private:
 
 TEST_F(FailPointStress, Basic) {
     startTest();
-    mongo::sleepsecs(30);
+    mongo::sleepsecs(5);
     stopTest();
 }
 
@@ -277,7 +278,7 @@ static int64_t runParallelFailPointTest(FailPoint::Mode fpMode,
                                         const int32_t numEncountersPerThread) {
     ASSERT_GT(numThreads, 0);
     ASSERT_GT(numEncountersPerThread, 0);
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(fpMode, fpVal);
     std::vector<stdx::thread*> tasks;
     std::vector<int64_t> counts(numThreads, 0);
@@ -398,7 +399,7 @@ TEST(FailPoint, parseBSONValidDataSucceeds) {
     ASSERT_TRUE(swTuple.isOK());
 }
 
-TEST(FailPoint, FailPointBlockBasicTest) {
+TEST(FailPoint, FailPointEnableBlockBasicTest) {
     auto failPoint = mongo::globalFailPointRegistry().find("dummy");
 
     ASSERT_FALSE(failPoint->shouldFail());
@@ -411,8 +412,21 @@ TEST(FailPoint, FailPointBlockBasicTest) {
     ASSERT_FALSE(failPoint->shouldFail());
 }
 
-TEST(FailPoint, FailPointBlockIfBasicTest) {
-    FailPoint failPoint;
+TEST(FailPoint, FailPointEnableBlockByPointer) {
+    auto failPoint = mongo::globalFailPointRegistry().find("dummy");
+
+    ASSERT_FALSE(failPoint->shouldFail());
+
+    {
+        FailPointEnableBlock dummyFp(failPoint);
+        ASSERT_TRUE(failPoint->shouldFail());
+    }
+
+    ASSERT_FALSE(failPoint->shouldFail());
+}
+
+TEST(FailPoint, ExecuteIfBasicTest) {
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::nTimes, 1, BSON("skip" << true));
     {
         bool hit = false;
@@ -463,7 +477,7 @@ void assertFunctionInterruptable(std::function<void(Interruptible* interruptible
 }
 
 TEST(FailPoint, PauseWhileSetInterruptibility) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn);
 
     assertFunctionInterruptable(
@@ -473,7 +487,7 @@ TEST(FailPoint, PauseWhileSetInterruptibility) {
 }
 
 TEST(FailPoint, WaitForFailPointTimeout) {
-    FailPoint failPoint;
+    FailPoint failPoint("testFP");
     failPoint.setMode(FailPoint::alwaysOn);
 
     assertFunctionInterruptable([&failPoint](Interruptible* interruptible) {