Merge pull request #356 from Expensify/tyler-fix-test-segfault

Fix segfault in parallel tests - reviewed, merging
Expensify · Jan 25, 2018 · 7607a1e · 7607a1e
2 parents 6c21fa8 + 6bf8274
commit 7607a1e
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 1 deletion.
diff --git a/test/lib/BedrockTester.cpp b/test/lib/BedrockTester.cpp
@@ -5,6 +5,7 @@
 string BedrockTester::defaultDBFile;
 string BedrockTester::defaultServerAddr;
 SData BedrockTester::globalArgs;
+mutex BedrockTester::_testersMutex;
 set<BedrockTester*> BedrockTester::_testers;
 list<string> BedrockTester::locations = {
     "../bedrock",
@@ -33,6 +34,7 @@ string BedrockTester::getServerName() {
 }
 
 void BedrockTester::stopAll() {
+    lock_guard<decltype(_testersMutex)> lock(_testersMutex);
     for (auto p : _testers) {
         p->stopServer();
     }
@@ -41,7 +43,10 @@ void BedrockTester::stopAll() {
 BedrockTester::BedrockTester(const map<string, string>& args, const list<string>& queries, bool startImmediately, bool keepFilesWhenFinished)
   : _keepFilesWhenFinished(keepFilesWhenFinished)
 {
-    _testers.insert(this);
+    {
+        lock_guard<decltype(_testersMutex)> lock(_testersMutex);
+        _testers.insert(this);
+    }
 
     // Set these values from the arguments if provided, or the defaults if not.
     try {
@@ -119,6 +124,7 @@ BedrockTester::~BedrockTester() {
         SFileExists((_dbName + "-shm").c_str()) && unlink((_dbName + "-shm").c_str());
         SFileExists((_dbName + "-wal").c_str()) && unlink((_dbName + "-wal").c_str());
     }
+    lock_guard<decltype(_testersMutex)> lock(_testersMutex);
     _testers.erase(this);
 }
 

diff --git a/test/lib/BedrockTester.h b/test/lib/BedrockTester.h
@@ -84,4 +84,7 @@ class BedrockTester {
 
     // A version of the DB that can be queries without going through bedrock.
     SQLite* _db = 0;
+
+    // For locking around changes to the _testers list.
+    static mutex _testersMutex;
 };