Merge pull request #1726 from Expensify/main

Update expensify_prod branch

BedrockPlugin.h

@@ -75,6 +75,9 @@ class BedrockPlugin {
     // Called when a node changes state
     virtual void stateChanged(SQLite& db, SQLiteNodeState newState) {}
 
+    // Called when the sync thread is finishing, before destroying DB handles.
+    virtual void serverStopping() {}
+
     // Map of plugin names to functions that will return a new plugin of the given type.
     static map<string, function<BedrockPlugin*(BedrockServer&)>> g_registeredPluginList;
 

BedrockServer.cpp

@@ -578,9 +578,7 @@ void BedrockServer::sync()
     // We just fell out of the loop where we were waiting for shutdown to complete. Update the state one last time when
     // the writing replication thread exits.
     if (getState() > SQLiteNodeState::WAITING) {
-        // This is because the graceful shutdown timer fired and syncNode.shutdownComplete() returned `true` above, but
-        // the server still thinks it's in some other state. We can only exit if we're in state <= SQLC_SEARCHING,
-        // (per BedrockServer::shutdownComplete()), so we force that state here to allow the shutdown to proceed.
+        // This should no longer be possible with fast shutdown.
         SWARN("Sync thread exiting in state " << SQLiteNode::stateName(getState()) << ".");
     }
 
@@ -606,17 +604,19 @@ void BedrockServer::sync()
         _blockingCommandQueue.clear();
     }
 
+    for (auto plugin : plugins) {
+        plugin.second->serverStopping();
+    }
+
     // We clear this before the _syncNode that it references.
     _clusterMessenger.reset();
 
     // Release our handle to this pointer. Any other functions that are still using it will keep the object alive
     // until they return.
     atomic_store(&_syncNode, shared_ptr<SQLiteNode>(nullptr));
 
-    // Release the current DB pool, and zero out our pointer.
-    // Note: This is not an atomic operation but should not matter. Nothing should use this that can happen with no
-    // sync thread.
-    // If there are socket threads in existance, they can be looking at this through a syncThread copy.
+    // Release the current DB pool, and zero out our pointer. If any socket threads hold a handle to `_syncNode`, they will keep this in existence
+    // until they release it.
     _dbPool = nullptr;
 
     // We're really done, store our flag so main() can be aware.

libstuff/libstuff.cpp

@@ -664,6 +664,16 @@ set<int64_t> SParseIntegerSet(const string& value, char separator) {
     return valueSet;
 }
 
+// --------------------------------------------------------------------------
+vector<int64_t> SParseIntegerVector(const string& value, char separator) {
+    vector<int64_t> valueVector;
+    list<string> strings = SParseList(value, separator);
+    for (const string& str : strings) {
+        valueVector.push_back(SToInt64(str));
+    }
+    return valueVector;
+}
+
 // --------------------------------------------------------------------------
 bool SParseList(const char* ptr, list<string>& valueList, char separator) {
     // Clear the input

libstuff/libstuff.h

@@ -446,6 +446,7 @@ string SEncodeURIComponent(const string& value);
 // List management
 list<int64_t> SParseIntegerList(const string& value, char separator = ',');
 set<int64_t> SParseIntegerSet(const string& value, char separator = ',');
+vector<int64_t> SParseIntegerVector(const string& value, char separator = ',');
 bool SParseList(const char* value, list<string>& valueList, char separator = ',');
 bool SParseList(const string& value, list<string>& valueList, char separator = ',');
 set<string> SParseSet(const string& value, char separator = ',');