From 646ea01f58fa1f590d3fa44253f83d8636e2422b Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Mon, 23 Mar 2026 13:25:19 -0400 Subject: [PATCH 1/9] Track dlopen handles and paths for dynamically loaded plugins Store the dlopen handle and filesystem path for each plugin loaded via shared library. These are saved in new static maps on BedrockPlugin (g_pluginDLHandles and g_pluginPaths), keyed by the upper-cased plugin name. This is prerequisite infrastructure for hot-reloading plugins without restarting Bedrock. Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockPlugin.cpp | 2 ++ BedrockPlugin.h | 4 ++++ main.cpp | 2 ++ 3 files changed, 8 insertions(+) diff --git a/BedrockPlugin.cpp b/BedrockPlugin.cpp index ca904c520..b1be951cf 100644 --- a/BedrockPlugin.cpp +++ b/BedrockPlugin.cpp @@ -3,6 +3,8 @@ #include "BedrockServer.h" map> BedrockPlugin::g_registeredPluginList; +map BedrockPlugin::g_pluginDLHandles; +map BedrockPlugin::g_pluginPaths; BedrockPlugin::BedrockPlugin(BedrockServer& s) : server(s) { diff --git a/BedrockPlugin.h b/BedrockPlugin.h index 11d7f2975..96100013a 100644 --- a/BedrockPlugin.h +++ b/BedrockPlugin.h @@ -101,6 +101,10 @@ class BedrockPlugin { // Map of plugin names to functions that will return a new plugin of the given type. static map> g_registeredPluginList; + // dlopen handles and filesystem paths for dynamically loaded plugins (keyed by UPPER(name)). + static map g_pluginDLHandles; + static map g_pluginPaths; + // Reference to the BedrockServer object that owns this plugin. BedrockServer& server; }; diff --git a/main.cpp b/main.cpp index 0eff7efc1..5a6064fa5 100644 --- a/main.cpp +++ b/main.cpp @@ -145,6 +145,8 @@ set loadPlugins(SData& args) } else { // Call the plugin registration function with the same name. BedrockPlugin::g_registeredPluginList.emplace(make_pair(SToUpper(name), (BedrockPlugin * (*)(BedrockServer&)) sym)); + BedrockPlugin::g_pluginDLHandles[SToUpper(name)] = lib; + BedrockPlugin::g_pluginPaths[SToUpper(name)] = pluginName; } } } From 1b689dd6146e49e759ebe71da592cd343aa5592c Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Mon, 23 Mar 2026 13:26:48 -0400 Subject: [PATCH 2/9] Add per-plugin active command counter Track in-flight commands per plugin via an atomic activeCommandCount on BedrockPlugin. Incremented in the BedrockCommand constructor and decremented in the destructor. This allows the ReloadPlugin handler to drain only the target plugin's commands rather than waiting for all commands globally. Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockCommand.cpp | 6 ++++++ BedrockPlugin.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/BedrockCommand.cpp b/BedrockCommand.cpp index 31ebbd37d..7a8405a9e 100644 --- a/BedrockCommand.cpp +++ b/BedrockCommand.cpp @@ -52,6 +52,9 @@ BedrockCommand::BedrockCommand(SQLiteCommand&& baseCommand, BedrockPlugin* plugi } } _commandCount++; + if (_plugin) { + _plugin->activeCommandCount++; + } } const string& BedrockCommand::getName() const @@ -88,6 +91,9 @@ BedrockCommand::~BedrockCommand() if (destructionCallback) { (*destructionCallback)(); } + if (_plugin) { + _plugin->activeCommandCount--; + } _commandCount--; } diff --git a/BedrockPlugin.h b/BedrockPlugin.h index 96100013a..e92373c8f 100644 --- a/BedrockPlugin.h +++ b/BedrockPlugin.h @@ -105,6 +105,9 @@ class BedrockPlugin { static map g_pluginDLHandles; static map g_pluginPaths; + // Tracks the number of in-flight commands owned by this plugin instance. + atomic activeCommandCount{0}; + // Reference to the BedrockServer object that owns this plugin. BedrockServer& server; }; From be6f25a290f525a1c690d7e42eff36db30697616 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Mon, 23 Mar 2026 13:27:47 -0400 Subject: [PATCH 3/9] Add ReloadPlugin control command for hot-swapping plugins Implement a new control port command that hot-reloads a dynamically loaded plugin (.so) without restarting Bedrock. The database stays open and mmap'd, the cluster membership persists, and only the plugin code is swapped. The reload follows a 7-phase sequence: 1. Validate the plugin exists and has a dlopen handle 2. Block the command port and reject new commands for the plugin 3. Drain in-flight commands (120s timeout, based on DEFAULT_TIMEOUT) 4. Destroy the old plugin instance and dlclose the old .so 5. dlopen the new .so and instantiate the new plugin 6. Run upgradeDatabase (if LEADING) and stateChanged 7. Unblock the command port On failure, attempts to roll back by re-loading the old .so. If rollback also fails, the plugin becomes unavailable but Bedrock continues serving other plugins. Also adds a shared_mutex (_pluginsMutex) to protect the plugins map during the brief swap window, with shared locks on all plugin iteration paths (getCommandFromPlugins, notifyStateChangeToPlugins, _upgradeDB, postPoll timer loop). Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockServer.cpp | 280 +++++++++++++++++++++++++++++++++++++++++++++- BedrockServer.h | 8 ++ 2 files changed, 284 insertions(+), 4 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index d22731ed2..4bb988223 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -7,7 +7,9 @@ #include #include #include +#include #include +#include #include #include @@ -1367,6 +1369,12 @@ BedrockServer::~BedrockServer() delete p.second; } + // Close any dynamically loaded plugin shared libraries. + for (auto& h : BedrockPlugin::g_pluginDLHandles) { + dlclose(h.second); + } + BedrockPlugin::g_pluginDLHandles.clear(); + shutdownTimer.serverDestructor = chrono::steady_clock::now(); SINFO("Shutdown timing: " << "start->safeState=" << chrono::duration_cast(shutdownTimer.safeNodeState - shutdownTimer.shutdownStart) @@ -1473,10 +1481,13 @@ void BedrockServer::postPoll(fd_map& fdm, uint64_t& nextActivity) _acceptSockets(); // If any plugin timers are firing, let the plugins know. - for (auto plugin : plugins) { - for (SStopwatch* timer : plugin.second->timers) { - if (timer->ding()) { - plugin.second->timerFired(timer); + { + shared_lock pluginLock(_pluginsMutex); + for (auto plugin : plugins) { + for (SStopwatch* timer : plugin.second->timers) { + if (timer->ding()) { + plugin.second->timerFired(timer); + } } } } @@ -1516,7 +1527,15 @@ unique_ptr BedrockServer::getCommandFromPlugins(SData&& request) unique_ptr BedrockServer::getCommandFromPlugins(unique_ptr&& baseCommand) { try { + shared_lock pluginLock(_pluginsMutex); for (auto pair : plugins) { + // Reject commands for a plugin that is being hot-reloaded. + if (_pluginReloadInProgress.load() && SIEquals(pair.first, _reloadingPluginName)) { + auto errorCommand = make_unique(SQLiteCommand(move(*baseCommand)), nullptr); + errorCommand->complete = true; + errorCommand->response.methodLine = "503 Plugin reload in progress"; + return errorCommand; + } // This is a bit weird to avoid changing this signature in all the plugins. It would be more straightforward if // the plugins just accepted a `unique_ptr&&`, but this still works. auto command = pair.second->getCommand(move(*baseCommand)); @@ -1857,6 +1876,7 @@ bool BedrockServer::_isControlCommand(const unique_ptr& command) SIEquals(command->request.methodLine, "BlockWrites") || SIEquals(command->request.methodLine, "UnblockWrites") || SIEquals(command->request.methodLine, "SetMaxSocketThreads") || + SIEquals(command->request.methodLine, "ReloadPlugin") || SIEquals(command->request.methodLine, "CRASH_COMMAND") ) { return true; @@ -2019,6 +2039,256 @@ void BedrockServer::_control(unique_ptr& command) } else { response.methodLine = "401 Don't Use Zero"; } + } else if (SIEquals(command->request.methodLine, "ReloadPlugin")) { + // Hot-reload a dynamically loaded plugin (.so) without restarting Bedrock. + string pluginKey = SToUpper(command->request["Plugin"]); + if (pluginKey.empty()) { + response.methodLine = "400 Missing Plugin header"; + return; + } + + // Phase 1: Validate + // The plugins map is keyed by plugin->getName() which may differ in case from the + // upper-cased key used in g_registeredPluginList/g_pluginDLHandles. Find it case-insensitively. + string pluginMapKey; + for (auto& p : plugins) { + if (SIEquals(p.first, pluginKey)) { + pluginMapKey = p.first; + break; + } + } + if (pluginMapKey.empty()) { + response.methodLine = "400 Plugin not found: " + pluginKey; + return; + } + auto handleIt = BedrockPlugin::g_pluginDLHandles.find(pluginKey); + if (handleIt == BedrockPlugin::g_pluginDLHandles.end()) { + response.methodLine = "400 Plugin is built-in, cannot reload: " + pluginKey; + return; + } + string pluginPath = BedrockPlugin::g_pluginPaths[pluginKey]; + if (!SFileExists(pluginPath)) { + response.methodLine = "400 Plugin .so file not found: " + pluginPath; + return; + } + + SINFO("[ReloadPlugin] Starting reload of plugin '" << pluginKey << "' from " << pluginPath); + + // Phase 2: Suppress new commands for this plugin + blockCommandPort("ReloadPlugin"); + _pluginReloadInProgress.store(true); + _reloadingPluginName = pluginMapKey; + + // Phase 3: Drain in-flight commands (120s timeout, slightly above DEFAULT_TIMEOUT of 110s) + constexpr uint64_t DRAIN_TIMEOUT_US = 120'000'000; + BedrockPlugin* oldPlugin = plugins[pluginMapKey]; + uint64_t drainStart = STimeNow(); + while (oldPlugin->activeCommandCount.load() > 0) { + if (STimeNow() - drainStart > DRAIN_TIMEOUT_US) { + SWARN("[ReloadPlugin] Drain timeout after 120s, " << oldPlugin->activeCommandCount.load() + << " commands still active. Aborting reload."); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: drain timeout"; + return; + } + usleep(10'000); + } + SINFO("[ReloadPlugin] All commands drained for plugin " << pluginKey); + + // Phase 4: Tear down old plugin + string oldSOPath = pluginPath; + void* oldHandle = handleIt->second; + string symbolName = "BEDROCK_PLUGIN_REGISTER_" + pluginKey; + + { + unique_lock pluginLock(_pluginsMutex); + oldPlugin->serverStopping(); + delete oldPlugin; + plugins.erase(pluginMapKey); + BedrockPlugin::g_pluginDLHandles.erase(pluginKey); + } + dlclose(oldHandle); + SINFO("[ReloadPlugin] Old plugin instance destroyed and .so closed"); + + // Phase 5: Load new plugin + void* newLib = dlopen(pluginPath.c_str(), RTLD_NOW); + if (!newLib) { + string dlErr = dlerror(); + SALERT("[ReloadPlugin] Failed to dlopen new .so: " << dlErr << ". Attempting rollback."); + // Attempt rollback with old path + void* rollbackLib = dlopen(oldSOPath.c_str(), RTLD_NOW); + if (rollbackLib) { + void* rollbackSym = dlsym(rollbackLib, symbolName.c_str()); + if (rollbackSym) { + try { + auto factory = (BedrockPlugin*(*)(BedrockServer&))rollbackSym; + BedrockPlugin* rollbackPlugin = factory(*this); + unique_lock pluginLock(_pluginsMutex); + plugins[pluginMapKey] = rollbackPlugin; + BedrockPlugin::g_pluginDLHandles[pluginKey] = rollbackLib; + BedrockPlugin::g_registeredPluginList[pluginKey] = factory; + pluginLock.unlock(); + { + SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); + rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + } + SWARN("[ReloadPlugin] Rolled back to previous plugin version"); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: " + dlErr + ". Rolled back to previous version."; + return; + } catch (const exception& e) { + dlclose(rollbackLib); + SALERT("[ReloadPlugin] Rollback constructor failed: " << e.what()); + } + } else { + dlclose(rollbackLib); + } + } + SALERT("[ReloadPlugin] Rollback failed. Plugin " << pluginKey << " is now unavailable."); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: " + dlErr + ". Rollback failed. Plugin " + pluginKey + " is now unavailable."; + return; + } + + void* newSym = dlsym(newLib, symbolName.c_str()); + if (!newSym) { + string dlErr = dlerror(); + dlclose(newLib); + SALERT("[ReloadPlugin] Failed to find symbol " << symbolName << ": " << dlErr << ". Attempting rollback."); + // Rollback attempt + void* rollbackLib = dlopen(oldSOPath.c_str(), RTLD_NOW); + if (rollbackLib) { + void* rollbackSym = dlsym(rollbackLib, symbolName.c_str()); + if (rollbackSym) { + try { + auto factory = (BedrockPlugin*(*)(BedrockServer&))rollbackSym; + BedrockPlugin* rollbackPlugin = factory(*this); + unique_lock pluginLock(_pluginsMutex); + plugins[pluginMapKey] = rollbackPlugin; + BedrockPlugin::g_pluginDLHandles[pluginKey] = rollbackLib; + BedrockPlugin::g_registeredPluginList[pluginKey] = factory; + pluginLock.unlock(); + { + SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); + rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + } + SWARN("[ReloadPlugin] Rolled back to previous plugin version"); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: " + dlErr + ". Rolled back to previous version."; + return; + } catch (const exception& e) { + dlclose(rollbackLib); + } + } else { + dlclose(rollbackLib); + } + } + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: " + dlErr + ". Rollback failed. Plugin " + pluginKey + " is now unavailable."; + return; + } + + auto newFactory = (BedrockPlugin*(*)(BedrockServer&))newSym; + BedrockPlugin* newPlugin = nullptr; + try { + newPlugin = newFactory(*this); + } catch (const exception& e) { + dlclose(newLib); + SALERT("[ReloadPlugin] New plugin constructor threw: " << e.what() << ". Attempting rollback."); + void* rollbackLib = dlopen(oldSOPath.c_str(), RTLD_NOW); + if (rollbackLib) { + void* rollbackSym = dlsym(rollbackLib, symbolName.c_str()); + if (rollbackSym) { + try { + auto factory = (BedrockPlugin*(*)(BedrockServer&))rollbackSym; + BedrockPlugin* rollbackPlugin = factory(*this); + unique_lock pluginLock(_pluginsMutex); + plugins[pluginMapKey] = rollbackPlugin; + BedrockPlugin::g_pluginDLHandles[pluginKey] = rollbackLib; + BedrockPlugin::g_registeredPluginList[pluginKey] = factory; + pluginLock.unlock(); + { + SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); + rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + } + SWARN("[ReloadPlugin] Rolled back to previous plugin version"); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: "s + e.what() + ". Rolled back to previous version."; + return; + } catch (const exception& e2) { + dlclose(rollbackLib); + } + } else { + dlclose(rollbackLib); + } + } + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: "s + e.what() + ". Rollback failed. Plugin " + pluginKey + " is now unavailable."; + return; + } + + // Insert the new plugin and save the handle + { + unique_lock pluginLock(_pluginsMutex); + plugins[pluginMapKey] = newPlugin; + } + BedrockPlugin::g_pluginDLHandles[pluginKey] = newLib; + BedrockPlugin::g_pluginPaths[pluginKey] = pluginPath; + BedrockPlugin::g_registeredPluginList[pluginKey] = newFactory; + SINFO("[ReloadPlugin] New plugin instance created and registered"); + + // Phase 6: Initialize new plugin + if (_dbPool) { + SQLiteScopedHandle dbScope(*_dbPool, _dbPool->getIndex()); + SQLite& db = dbScope.db(); + if (getState() == SQLiteNodeState::LEADING) { + SINFO("[ReloadPlugin] Running upgradeDatabase for " << pluginKey); + try { + db.beginTransaction(SQLite::TRANSACTION_TYPE::EXCLUSIVE); + newPlugin->upgradeDatabase(db); + if (db.getUncommittedQuery().empty()) { + db.rollback(); + } else { + SINFO("[ReloadPlugin] Schema changes detected, committing."); + db.prepare(); + db.commit(); + } + } catch (const exception& e) { + SWARN("[ReloadPlugin] upgradeDatabase failed: " << e.what()); + db.rollback(); + } + } + newPlugin->stateChanged(db, getState()); + } + + // Update the version string + STable pluginInfo = newPlugin->getInfo(); + if (!pluginInfo.empty()) { + for (auto& row : pluginInfo) { + SINFO("[ReloadPlugin] New plugin info: " << row.first << " = " << row.second); + } + } + + // Phase 7: Resume + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + SINFO("[ReloadPlugin] Plugin " << pluginKey << " reloaded successfully"); + response.methodLine = "200 Plugin reloaded successfully"; } } @@ -2026,6 +2296,7 @@ bool BedrockServer::_upgradeDB(SQLite& db) { // These all get conglomerated into one big query. try { + shared_lock pluginLock(_pluginsMutex); db.beginTransaction(SQLite::TRANSACTION_TYPE::EXCLUSIVE); for (auto plugin : plugins) { plugin.second->upgradeDatabase(db); @@ -2549,6 +2820,7 @@ void BedrockServer::handleSocket(Socket&& socket, bool fromControlPort, bool fro void BedrockServer::notifyStateChangeToPlugins(SQLite& db, SQLiteNodeState newState) { + shared_lock pluginLock(_pluginsMutex); for (auto plugin : plugins) { plugin.second->stateChanged(db, newState); } diff --git a/BedrockServer.h b/BedrockServer.h index 622c73ffb..ac26734ac 100644 --- a/BedrockServer.h +++ b/BedrockServer.h @@ -1,5 +1,6 @@ #pragma once #include +#include #include #include #include @@ -437,6 +438,13 @@ class BedrockServer : public SQLiteServer { // Whether or not all plugins are detached bool _pluginsDetached; + // Plugin hot-reload support. _pluginReloadInProgress is checked by getCommandFromPlugins() to reject + // commands for the plugin being reloaded. _reloadingPluginName is the UPPER-cased name of that plugin. + // _pluginsMutex protects the plugins map during the brief swap window. + atomic _pluginReloadInProgress{false}; + string _reloadingPluginName; + shared_mutex _pluginsMutex; + // This is a snapshot of the state of the node taken at the beginning of any call to peekCommand or processCommand // so that the state can't change for the lifetime of that call, from the view of that function. static thread_local atomic _nodeStateSnapshot; From d9b5cafad213ba7f024a091d91dee8dcfad1b968 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Mon, 23 Mar 2026 13:27:59 -0400 Subject: [PATCH 4/9] Add ReloadPlugin cluster test suite Test coverage for the ReloadPlugin control command: - Happy path reload with command verification - Drain of in-flight commands before reload completes - Rejection of built-in plugin reload (DB) - Rejection of nonexistent plugin reload - Database state preservation across reloads - Follower node reload - Serialization of concurrent reload requests Co-Authored-By: Claude Opus 4.6 (1M context) --- test/clustertest/tests/ReloadPluginTest.cpp | 151 ++++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 test/clustertest/tests/ReloadPluginTest.cpp diff --git a/test/clustertest/tests/ReloadPluginTest.cpp b/test/clustertest/tests/ReloadPluginTest.cpp new file mode 100644 index 000000000..f21c59256 --- /dev/null +++ b/test/clustertest/tests/ReloadPluginTest.cpp @@ -0,0 +1,151 @@ +#include +#include +#include + +struct ReloadPluginTest : tpunit::TestFixture { + ReloadPluginTest() + : tpunit::TestFixture("ReloadPlugin", + TEST(ReloadPluginTest::happyPathReload), + TEST(ReloadPluginTest::reloadDrainsInflightCommands), + TEST(ReloadPluginTest::reloadBuiltinPluginRejected), + TEST(ReloadPluginTest::reloadNonexistentPluginRejected), + TEST(ReloadPluginTest::reloadPreservesDBState), + TEST(ReloadPluginTest::reloadOnFollower), + TEST(ReloadPluginTest::doubleReloadSerializes)) + { + } + + // Helper to send ReloadPlugin control command + string sendReloadPlugin(BedrockTester& tester, const string& pluginName) { + SData command("ReloadPlugin"); + command["Plugin"] = pluginName; + return tester.executeWaitVerifyContent(command, "", true); + } + + void happyPathReload() { + // Start a 3-node cluster with the test plugin + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + // Verify plugin works before reload + SData cmd("testcommand"); + leader.executeWaitVerifyContent(cmd, "200"); + + // Reload the plugin + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + leader.executeWaitVerifyContent(reloadCmd, "200", true); + + // Verify plugin still works after reload + SData cmd2("testcommand"); + leader.executeWaitVerifyContent(cmd2, "200"); + } + + void reloadDrainsInflightCommands() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + // Start a slow query that will take a few seconds + SData slowCmd("slowquery"); + slowCmd["size"] = "5000000"; + + // Send the slow command in a background thread + thread slowThread([&]() { + leader.executeWaitVerifyContent(slowCmd, "200"); + }); + + // Give it a moment to start processing + usleep(100'000); + + // Send reload - it should wait for the slow query to finish + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + leader.executeWaitVerifyContent(reloadCmd, "200", true); + + slowThread.join(); + + // Verify commands work after reload + SData cmd("testcommand"); + leader.executeWaitVerifyContent(cmd, "200"); + } + + void reloadBuiltinPluginRejected() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + SData cmd("ReloadPlugin"); + cmd["Plugin"] = "DB"; + leader.executeWaitVerifyContent(cmd, "400", true); + } + + void reloadNonexistentPluginRejected() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + SData cmd("ReloadPlugin"); + cmd["Plugin"] = "FAKEPLUGIN"; + leader.executeWaitVerifyContent(cmd, "400", true); + } + + void reloadPreservesDBState() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + // Insert some data + SData insertCmd("testquery"); + insertCmd["Query"] = "INSERT INTO test (id, value) VALUES (12345, 'reload_test_data');"; + leader.executeWaitVerifyContent(insertCmd, "200"); + + // Reload the plugin + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + leader.executeWaitVerifyContent(reloadCmd, "200", true); + + // Verify data persists after reload + SData selectCmd("testquery"); + selectCmd["Query"] = "SELECT value FROM test WHERE id = 12345;"; + leader.executeWaitVerifyContent(selectCmd, "200"); + } + + void reloadOnFollower() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& follower = tester.getTester(1); + + // Wait for the follower to be in FOLLOWING state + follower.waitForState("FOLLOWING"); + + // Reload on the follower + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + follower.executeWaitVerifyContent(reloadCmd, "200", true); + + // Verify commands still work on the follower + SData cmd("testcommand"); + follower.executeWaitVerifyContent(cmd, "200"); + } + + void doubleReloadSerializes() { + BedrockClusterTester tester(ClusterSize::THREE_NODE_CLUSTER); + BedrockTester& leader = tester.getTester(0); + + // Send two reload commands concurrently + thread t1([&]() { + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + leader.executeWaitVerifyContent(reloadCmd, "", true); + }); + thread t2([&]() { + SData reloadCmd("ReloadPlugin"); + reloadCmd["Plugin"] = "TESTPLUGIN"; + leader.executeWaitVerifyContent(reloadCmd, "", true); + }); + + t1.join(); + t2.join(); + + // Server should still be stable + SData cmd("testcommand"); + leader.executeWaitVerifyContent(cmd, "200"); + } + +} __ReloadPluginTest; From fd7a0b3a0bdd16a702943884667947c69ec1f0ed Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Mon, 23 Mar 2026 13:50:22 -0400 Subject: [PATCH 5/9] Add Plugin to log params whitelist for ReloadPlugin command The ReloadPlugin control command uses a "Plugin" header to specify which plugin to reload. Without whitelisting this param name, the logging system throws an exception in dev mode when trying to log the response, crashing the process. Co-Authored-By: Claude Opus 4.6 (1M context) --- libstuff/SLog.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index 9eb260a44..360425545 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -62,6 +62,7 @@ static set PARAMS_WHITELIST = { "logParam", "message", "peer", + "Plugin", "prepareElapsed", "query", "readElapsed", From 3eb49f42679ef0206e2df0b664233470537b6347 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Tue, 24 Mar 2026 11:09:40 -0400 Subject: [PATCH 6/9] Fix ReloadPlugin crash: rename header and resolve .so path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues caused a crash when sending ReloadPlugin: 1. The "Plugin" header name collided with the internal "plugin" header that _reply() uses to route responses to plugin port handlers. SData uses case-insensitive key comparison, so request["Plugin"] matched request["plugin"]. When _reply() found this non-empty value, it tried to look up the plugin by name in the plugins map, failed, and hit SERROR (abort). Fix: rename the header to "PluginName" which does not collide. 2. The stored .so path was the bare filename passed to -plugins (e.g., "auth.so"), not the resolved absolute path. SFileExists failed because "auth.so" doesn't exist relative to CWD — dlopen found it via /usr/lib. Fix: use dlinfo(RTLD_DI_LINKMAP) after dlopen to resolve the actual filesystem path, then realpath() to follow symlinks. Also updates the log params whitelist and test suite to use "PluginName". Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockServer.cpp | 6 ++++-- libstuff/SLog.cpp | 2 +- main.cpp | 18 +++++++++++++++++- test/clustertest/tests/ReloadPluginTest.cpp | 18 +++++++++--------- 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 4bb988223..0dcd552a5 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2041,9 +2041,11 @@ void BedrockServer::_control(unique_ptr& command) } } else if (SIEquals(command->request.methodLine, "ReloadPlugin")) { // Hot-reload a dynamically loaded plugin (.so) without restarting Bedrock. - string pluginKey = SToUpper(command->request["Plugin"]); + // Use "PluginName" header (not "Plugin") to avoid collision with the internal "plugin" + // header that _reply() uses to route responses to plugin port handlers. + string pluginKey = SToUpper(command->request["PluginName"]); if (pluginKey.empty()) { - response.methodLine = "400 Missing Plugin header"; + response.methodLine = "400 Missing PluginName header"; return; } diff --git a/libstuff/SLog.cpp b/libstuff/SLog.cpp index 360425545..771f64982 100644 --- a/libstuff/SLog.cpp +++ b/libstuff/SLog.cpp @@ -62,7 +62,7 @@ static set PARAMS_WHITELIST = { "logParam", "message", "peer", - "Plugin", + "PluginName", "prepareElapsed", "query", "readElapsed", diff --git a/main.cpp b/main.cpp index 5a6064fa5..97a168b3f 100644 --- a/main.cpp +++ b/main.cpp @@ -4,6 +4,7 @@ /// #include #include +#include #include #include #include @@ -146,7 +147,22 @@ set loadPlugins(SData& args) // Call the plugin registration function with the same name. BedrockPlugin::g_registeredPluginList.emplace(make_pair(SToUpper(name), (BedrockPlugin * (*)(BedrockServer&)) sym)); BedrockPlugin::g_pluginDLHandles[SToUpper(name)] = lib; - BedrockPlugin::g_pluginPaths[SToUpper(name)] = pluginName; + + // Resolve the full filesystem path of the loaded .so via dlinfo, since + // pluginName may be a bare filename (e.g., "auth.so") that dlopen resolved + // through the standard library search path. + string resolvedPath = pluginName; + struct link_map* lm = nullptr; + if (dlinfo(lib, RTLD_DI_LINKMAP, &lm) == 0 && lm && lm->l_name[0]) { + char* rp = realpath(lm->l_name, nullptr); + if (rp) { + resolvedPath = rp; + free(rp); + } else { + resolvedPath = lm->l_name; + } + } + BedrockPlugin::g_pluginPaths[SToUpper(name)] = resolvedPath; } } } diff --git a/test/clustertest/tests/ReloadPluginTest.cpp b/test/clustertest/tests/ReloadPluginTest.cpp index f21c59256..a922dae29 100644 --- a/test/clustertest/tests/ReloadPluginTest.cpp +++ b/test/clustertest/tests/ReloadPluginTest.cpp @@ -18,7 +18,7 @@ struct ReloadPluginTest : tpunit::TestFixture { // Helper to send ReloadPlugin control command string sendReloadPlugin(BedrockTester& tester, const string& pluginName) { SData command("ReloadPlugin"); - command["Plugin"] = pluginName; + command["PluginName"] = pluginName; return tester.executeWaitVerifyContent(command, "", true); } @@ -33,7 +33,7 @@ struct ReloadPluginTest : tpunit::TestFixture { // Reload the plugin SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; leader.executeWaitVerifyContent(reloadCmd, "200", true); // Verify plugin still works after reload @@ -59,7 +59,7 @@ struct ReloadPluginTest : tpunit::TestFixture { // Send reload - it should wait for the slow query to finish SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; leader.executeWaitVerifyContent(reloadCmd, "200", true); slowThread.join(); @@ -74,7 +74,7 @@ struct ReloadPluginTest : tpunit::TestFixture { BedrockTester& leader = tester.getTester(0); SData cmd("ReloadPlugin"); - cmd["Plugin"] = "DB"; + cmd["PluginName"] = "DB"; leader.executeWaitVerifyContent(cmd, "400", true); } @@ -83,7 +83,7 @@ struct ReloadPluginTest : tpunit::TestFixture { BedrockTester& leader = tester.getTester(0); SData cmd("ReloadPlugin"); - cmd["Plugin"] = "FAKEPLUGIN"; + cmd["PluginName"] = "FAKEPLUGIN"; leader.executeWaitVerifyContent(cmd, "400", true); } @@ -98,7 +98,7 @@ struct ReloadPluginTest : tpunit::TestFixture { // Reload the plugin SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; leader.executeWaitVerifyContent(reloadCmd, "200", true); // Verify data persists after reload @@ -116,7 +116,7 @@ struct ReloadPluginTest : tpunit::TestFixture { // Reload on the follower SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; follower.executeWaitVerifyContent(reloadCmd, "200", true); // Verify commands still work on the follower @@ -131,12 +131,12 @@ struct ReloadPluginTest : tpunit::TestFixture { // Send two reload commands concurrently thread t1([&]() { SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; leader.executeWaitVerifyContent(reloadCmd, "", true); }); thread t2([&]() { SData reloadCmd("ReloadPlugin"); - reloadCmd["Plugin"] = "TESTPLUGIN"; + reloadCmd["PluginName"] = "TESTPLUGIN"; leader.executeWaitVerifyContent(reloadCmd, "", true); }); From 8145b3c7513ed1dca3f9f1f648b82bcf693e0d01 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Tue, 24 Mar 2026 11:53:11 -0400 Subject: [PATCH 7/9] Fix stateChanged DB handle lifetime for plugin reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auth's stateChanged() spawns a detached thread that captures the SQLite& db reference and uses it asynchronously (waiting for isUpgradeComplete(), then querying maxOnyxUpdateID). When called from the ReloadPlugin handler, the db reference came from a SQLiteScopedHandle that was destroyed at the end of the block, leaving the detached thread with a dangling reference → segfault. Fix: acquire a DB handle from the pool without scoping it, call stateChanged, then spawn a background thread that waits for isUpgradeComplete() before returning the handle to the pool. This ensures the db reference remains valid for the full duration of the plugin's async initialization. Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockServer.cpp | 60 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 0dcd552a5..1021b8b19 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2133,8 +2133,16 @@ void BedrockServer::_control(unique_ptr& command) BedrockPlugin::g_registeredPluginList[pluginKey] = factory; pluginLock.unlock(); { - SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); - rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + size_t rbIdx = _dbPool->getIndex(); + SQLite& rbDB = _dbPool->initializeIndex(rbIdx); + rollbackPlugin->stateChanged(rbDB, getState()); + shared_ptr rbPoolCopy = _dbPool; + thread([rbPoolCopy, rbIdx, this]() { + SInitialize("ReloadPluginRollbackDBReturn"); + while (!isUpgradeComplete()) { usleep(50'000); } + usleep(100'000); + rbPoolCopy->returnToPool(rbIdx); + }).detach(); } SWARN("[ReloadPlugin] Rolled back to previous plugin version"); _pluginReloadInProgress.store(false); @@ -2177,8 +2185,16 @@ void BedrockServer::_control(unique_ptr& command) BedrockPlugin::g_registeredPluginList[pluginKey] = factory; pluginLock.unlock(); { - SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); - rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + size_t rbIdx = _dbPool->getIndex(); + SQLite& rbDB = _dbPool->initializeIndex(rbIdx); + rollbackPlugin->stateChanged(rbDB, getState()); + shared_ptr rbPoolCopy = _dbPool; + thread([rbPoolCopy, rbIdx, this]() { + SInitialize("ReloadPluginRollbackDBReturn"); + while (!isUpgradeComplete()) { usleep(50'000); } + usleep(100'000); + rbPoolCopy->returnToPool(rbIdx); + }).detach(); } SWARN("[ReloadPlugin] Rolled back to previous plugin version"); _pluginReloadInProgress.store(false); @@ -2220,8 +2236,16 @@ void BedrockServer::_control(unique_ptr& command) BedrockPlugin::g_registeredPluginList[pluginKey] = factory; pluginLock.unlock(); { - SQLiteScopedHandle rbDBScope(*_dbPool, _dbPool->getIndex()); - rollbackPlugin->stateChanged(rbDBScope.db(), getState()); + size_t rbIdx = _dbPool->getIndex(); + SQLite& rbDB = _dbPool->initializeIndex(rbIdx); + rollbackPlugin->stateChanged(rbDB, getState()); + shared_ptr rbPoolCopy = _dbPool; + thread([rbPoolCopy, rbIdx, this]() { + SInitialize("ReloadPluginRollbackDBReturn"); + while (!isUpgradeComplete()) { usleep(50'000); } + usleep(100'000); + rbPoolCopy->returnToPool(rbIdx); + }).detach(); } SWARN("[ReloadPlugin] Rolled back to previous plugin version"); _pluginReloadInProgress.store(false); @@ -2254,9 +2278,14 @@ void BedrockServer::_control(unique_ptr& command) SINFO("[ReloadPlugin] New plugin instance created and registered"); // Phase 6: Initialize new plugin + // Auth's stateChanged() spawns a detached thread that captures `db` by reference + // and uses it after stateChanged() returns. A SQLiteScopedHandle would be destroyed + // at the end of this block, leaving the detached thread with a dangling reference. + // Instead, we acquire a pool handle and return it in a background thread after + // the plugin's async initialization completes. if (_dbPool) { - SQLiteScopedHandle dbScope(*_dbPool, _dbPool->getIndex()); - SQLite& db = dbScope.db(); + size_t dbIndex = _dbPool->getIndex(); + SQLite& db = _dbPool->initializeIndex(dbIndex); if (getState() == SQLiteNodeState::LEADING) { SINFO("[ReloadPlugin] Running upgradeDatabase for " << pluginKey); try { @@ -2275,6 +2304,21 @@ void BedrockServer::_control(unique_ptr& command) } } newPlugin->stateChanged(db, getState()); + + // Return the DB handle to the pool once the plugin's async initialization is done. + // This is necessary because Auth's stateChanged spawns a detached thread that uses + // the db reference until isUpgradeComplete() returns true. + shared_ptr dbPoolCopy = _dbPool; + thread([dbPoolCopy, dbIndex, this]() { + SInitialize("ReloadPluginDBReturn"); + while (!isUpgradeComplete()) { + usleep(50'000); + } + // Give the plugin's thread a moment to finish using the handle after upgrade completes. + usleep(100'000); + dbPoolCopy->returnToPool(dbIndex); + SINFO("[ReloadPlugin] Returned DB handle to pool after plugin initialization"); + }).detach(); } // Update the version string From c55922b66d8d6706cdd8073bd3cb289507c66ac9 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Tue, 24 Mar 2026 12:01:55 -0400 Subject: [PATCH 8/9] Force fresh dlopen by copying .so to a temp path before loading dlopen caches loaded libraries by path. Calling dlclose() followed by dlopen() on the same path returns the old in-memory copy rather than re-reading from disk. This meant ReloadPlugin appeared to succeed but actually loaded the same code. Fix: copy the .so to a unique temporary path (appending a timestamp) before calling dlopen, then delete the temp file. Since the library is already mapped into memory after dlopen, deleting the file is safe. This forces the dynamic linker to load fresh code from disk on every reload. Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockServer.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 1021b8b19..6fe4a96ef 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2115,7 +2115,19 @@ void BedrockServer::_control(unique_ptr& command) SINFO("[ReloadPlugin] Old plugin instance destroyed and .so closed"); // Phase 5: Load new plugin - void* newLib = dlopen(pluginPath.c_str(), RTLD_NOW); + // dlopen caches by path — calling dlclose + dlopen on the same path may return the + // old in-memory copy. Copy the .so to a unique temp path to force a fresh load. + string tempSOPath = pluginPath + ".reload." + to_string(STimeNow()); + if (!SFileCopy(pluginPath, tempSOPath)) { + SALERT("[ReloadPlugin] Failed to copy " << pluginPath << " to " << tempSOPath); + _pluginReloadInProgress.store(false); + _reloadingPluginName.clear(); + unblockCommandPort("ReloadPlugin"); + response.methodLine = "500 Plugin reload failed: could not create temp copy of .so"; + return; + } + void* newLib = dlopen(tempSOPath.c_str(), RTLD_NOW); + SFileDelete(tempSOPath); if (!newLib) { string dlErr = dlerror(); SALERT("[ReloadPlugin] Failed to dlopen new .so: " << dlErr << ". Attempting rollback."); From 0f287dfcc52f424cf05be9ef74f6499c31a8f414 Mon Sep 17 00:00:00 2001 From: Justin Persaud Date: Tue, 24 Mar 2026 12:11:44 -0400 Subject: [PATCH 9/9] Rebuild _version string after plugin reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The top-level version string reported by the Status command was not updated after a plugin reload — it still contained the old plugin version hash. Rebuild it from all current plugins after each successful reload so the version string stays consistent with the per-plugin version data. Co-Authored-By: Claude Opus 4.6 (1M context) --- BedrockServer.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/BedrockServer.cpp b/BedrockServer.cpp index 6fe4a96ef..6c1681bfd 100644 --- a/BedrockServer.cpp +++ b/BedrockServer.cpp @@ -2333,12 +2333,19 @@ void BedrockServer::_control(unique_ptr& command) }).detach(); } - // Update the version string - STable pluginInfo = newPlugin->getInfo(); - if (!pluginInfo.empty()) { - for (auto& row : pluginInfo) { - SINFO("[ReloadPlugin] New plugin info: " << row.first << " = " << row.second); + // Rebuild the _version string to reflect the reloaded plugin's version. + { + vector versions = {VERSION}; + for (auto& p : plugins) { + auto info = p.second->getInfo(); + auto it = info.find("version"); + if (it != info.end()) { + versions.push_back(p.second->getName() + "_" + it->second); + } } + sort(versions.begin(), versions.end()); + _version = SComposeList(versions, ":"); + SINFO("[ReloadPlugin] Updated version string: " << _version); } // Phase 7: Resume