Merge pull request #1826 from Expensify/dan/prevent-double-detach-crash

Do not crash if the detach command is sent twice.

BedrockServer.cpp

@@ -1798,8 +1798,12 @@ void BedrockServer::_control(unique_ptr<BedrockCommand>& command) {
     } else if (SIEquals(command->request.methodLine, "ConflictReport")) {
         response.content = _conflictManager.generateReport();
     } else if (SIEquals(command->request.methodLine, "Detach")) {
-        response.methodLine = "203 DETACHING";
-        _beginShutdown("Detach", true);
+        if (isDetached()) {
+            response.methodLine = "400 Already detached";
+        } else {
+            response.methodLine = "203 DETACHING";
+            _beginShutdown("Detach", true);
+        }
     } else if (SIEquals(command->request.methodLine, "Attach")) {
         // Ensure none of our plugins are blocking attaching
         list<string> blockingPlugins;

test/clustertest/tests/DoubleDetachTest.cpp

@@ -0,0 +1,38 @@
+#include <iostream>
+
+#include <libstuff/SData.h>
+#include <test/clustertest/BedrockClusterTester.h>
+
+struct DoubleDetachTest : tpunit::TestFixture {
+    DoubleDetachTest()
+        : tpunit::TestFixture("DoubleDetach",
+                              BEFORE_CLASS(DoubleDetachTest::setup),
+                              AFTER_CLASS(DoubleDetachTest::teardown),
+                              TEST(DoubleDetachTest::testDoubleDetach)) { }
+
+    BedrockClusterTester* tester;
+
+    void setup() {
+        tester = new BedrockClusterTester();
+    }
+
+    void teardown() {
+        delete tester;
+    }
+
+    void testDoubleDetach()
+    {
+        // Test a control command
+        BedrockTester& follower = tester->getTester(1);
+
+        // Detach
+        SData detachCommand("detach");
+        follower.executeWaitVerifyContent(detachCommand, "203 DETACHING", true);
+
+        // Wait for it to detach
+        sleep(3);
+
+        follower.executeWaitVerifyContent(detachCommand, "400 Already detached", true);
+    }
+
+} __DoubleDetachTest;