From 894756b65c9aaac7dde73252cd76aaf4bed19385 Mon Sep 17 00:00:00 2001 From: defnax <9952056+defnax@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:33:25 +0200 Subject: [PATCH 1/7] Changes for JSONAPI to work with plugins --- src/retroshare/rsplugin.h | 4 +++- src/rsserver/rsinit.cc | 24 ++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/retroshare/rsplugin.h b/src/retroshare/rsplugin.h index f7cec2b52..0939edd5c 100644 --- a/src/retroshare/rsplugin.h +++ b/src/retroshare/rsplugin.h @@ -38,6 +38,7 @@ extern RsPluginHandler *rsPlugins ; class p3Service ; class RsServiceControl ; class RsReputations ; +class RsJsonApi; class RsTurtle ; class RsGxsTunnelService ; class RsDht ; @@ -90,7 +91,7 @@ namespace resource_api // Plugin API version. Not used yet, but will be in the future the // main value that decides for compatibility. // -#define RS_PLUGIN_API_VERSION 0x000101 +#define RS_PLUGIN_API_VERSION 0x000102 // Used for the status of plugins. // @@ -122,6 +123,7 @@ public: RsUtil::inited_ptr mDht; RsUtil::inited_ptr mServiceControl; RsUtil::inited_ptr mPluginHandler; + RsUtil::inited_ptr mJsonApi; // gxs std::string mGxsDir; diff --git a/src/rsserver/rsinit.cc b/src/rsserver/rsinit.cc index 31d6e08c4..711c35fd7 100644 --- a/src/rsserver/rsinit.cc +++ b/src/rsserver/rsinit.cc @@ -445,8 +445,18 @@ void RsInit::startupWebServices(const RsConfigOptions& conf,bool force_start_jso std::cerr << std::endl; RsInfo() << "Configuring web services" ; - JsonApiServer* jas = new JsonApiServer(); - bool jsonapi_needed = force_start_jsonapi; + // Plugins receive RsPlugInInterfaces during core startup, before the GUI + // calls startupWebServices(). Reuse the server created for that handoff so + // resource providers registered by plugins are not lost here. + JsonApiServer* jas = dynamic_cast(rsJsonApi); + if(!jas) + { + jas = new JsonApiServer(); + rsJsonApi = jas; + } + // A plugin provider also requires the server even when the built-in WebUI + // is disabled. Providers were registered during plugin initialization. + bool jsonapi_needed = force_start_jsonapi || !jas->getResourceProviders().empty(); // add jsonapi server to config manager so that it can save/load its tokens p3ConfigMgr *cfgmgr = dynamic_cast(RsControl::instance()->configManager()); @@ -1675,6 +1685,16 @@ int RsServer::StartupRetroShare() #endif interfaces.mServiceControl = serviceCtrl; interfaces.mPluginHandler = mPluginsManager; +#ifdef RS_JSONAPI + // The GUI configures web services after plugins are initialized. Create + // the server now so plugins can register providers in setInterfaces(). + // startupWebServices() will configure and reuse this same instance later. + if(!rsJsonApi) + rsJsonApi = new JsonApiServer(); + interfaces.mJsonApi = rsJsonApi; +#else + interfaces.mJsonApi = nullptr; +#endif // gxs interfaces.mGxsDir = currGxsDir; interfaces.mIdentity = mGxsIdService; From fc6ee7fb6bda02967cfe294f315d37e95090e4cc Mon Sep 17 00:00:00 2001 From: defnax <9952056+defnax@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:41:51 +0200 Subject: [PATCH 2/7] Added requested changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Moved mJsonApi to the end of RsPlugInInterfaces. - Enforced RS_PLUGIN_API_VERSION; incompatible plugins now get PLUGIN_STATUS_WRONG_API. - Added a GUI message for incompatible plugins. - Removed automatic JSON API startup based only on provider presence. - Removed FeedReader’s per-plugin startup restart. - Core now restarts JSON API once after all plugins register, only when already running. - Added mutex protection and provider snapshots for mResourceProviders. --- src/jsonapi/jsonapi.cpp | 23 +++++++++++++++++------ src/jsonapi/jsonapi.h | 5 +++-- src/plugins/pluginmanager.cc | 12 +++++++++++- src/retroshare/rsjsonapi.h | 6 +++++- src/retroshare/rsplugin.h | 8 +++++--- src/rsserver/rsinit.cc | 25 +++++++++++++++++++++---- 6 files changed, 62 insertions(+), 17 deletions(-) diff --git a/src/jsonapi/jsonapi.cpp b/src/jsonapi/jsonapi.cpp index 2d91ee36f..2a1abf7f4 100644 --- a/src/jsonapi/jsonapi.cpp +++ b/src/jsonapi/jsonapi.cpp @@ -155,6 +155,7 @@ bool RsJsonApi::parseToken( } JsonApiServer::JsonApiServer(): configMutex("JsonApiServer config"), + mResourceProvidersMutex("JsonApiServer resource providers"), mService(nullptr), mListeningPort(RsJsonApi::DEFAULT_PORT), mBindingAddress(RsJsonApi::DEFAULT_BINDING_ADDRESS), @@ -836,7 +837,10 @@ void JsonApiServer::handleCorsOptions( void JsonApiServer::registerResourceProvider(const JsonApiResourceProvider& rp) { - mResourceProviders.insert(rp); + { + RS_STACK_MUTEX(mResourceProvidersMutex); + mResourceProviders.insert(rp); + } if(rsEvents) { @@ -847,7 +851,10 @@ void JsonApiServer::registerResourceProvider(const JsonApiResourceProvider& rp) } void JsonApiServer::unregisterResourceProvider(const JsonApiResourceProvider& rp) { - mResourceProviders.erase(rp); + { + RS_STACK_MUTEX(mResourceProvidersMutex); + mResourceProviders.erase(rp); + } if(rsEvents) { @@ -857,10 +864,14 @@ void JsonApiServer::unregisterResourceProvider(const JsonApiResourceProvider& rp } } bool JsonApiServer::hasResourceProvider(const JsonApiResourceProvider& rp) -{ return mResourceProviders.find(rp) != mResourceProviders.end(); } - -const std::set,std::less >& JsonApiServer::getResourceProviders() const { + RS_STACK_MUTEX(mResourceProvidersMutex); + return mResourceProviders.find(rp) != mResourceProviders.end(); +} + +RsJsonApi::ResourceProviderSet JsonApiServer::getResourceProviders() const +{ + RS_STACK_MUTEX(mResourceProvidersMutex); return mResourceProviders; } @@ -868,7 +879,7 @@ std::vector > JsonApiServer::getResources() const { auto tab = mResources; - for(auto& rp: mResourceProviders) + for(auto& rp: getResourceProviders()) for(auto r: rp.get().getResources()) tab.push_back(r); return tab; diff --git a/src/jsonapi/jsonapi.h b/src/jsonapi/jsonapi.h index d4bd90bbc..48a66bd39 100644 --- a/src/jsonapi/jsonapi.h +++ b/src/jsonapi/jsonapi.h @@ -64,8 +64,8 @@ public: std::vector> getResources() const; - /// @see RsJsonApi - const std::set,std::less >& getResourceProviders() const override; + /// @see RsJsonApi + ResourceProviderSet getResourceProviders() const override; /// @see RsJsonApi void fullstop() override { RsThread::fullstop(); } @@ -180,6 +180,7 @@ private: /// Encrypted persistent storage for authorized JSON API tokens JsonApiServerAuthTokenStorage mAuthTokenStorage; RsMutex configMutex; + mutable RsMutex mResourceProvidersMutex; static const std::multimap corsHeaders; static const std::multimap corsOptionsHeaders; diff --git a/src/plugins/pluginmanager.cc b/src/plugins/pluginmanager.cc index 70371360e..99b333c7c 100644 --- a/src/plugins/pluginmanager.cc +++ b/src/plugins/pluginmanager.cc @@ -371,7 +371,17 @@ bool RsPluginManager::loadPlugin(const std::string& plugin_name,bool first_time) pinfo.info_string = "" ; dlclose(handle); return false ; - } + } + if(pinfo.API_version != RS_PLUGIN_API_VERSION) + { + std::cerr << " -> Incompatible plugin API version " + << std::hex << pinfo.API_version << "; expected " + << RS_PLUGIN_API_VERSION << std::dec << std::endl; + pinfo.status = PLUGIN_STATUS_WRONG_API; + pinfo.info_string = "Incompatible plugin API version"; + dlclose(handle); + return false; + } #ifdef TO_REMOVE if(pinfo.svn_revision == 0) { diff --git a/src/retroshare/rsjsonapi.h b/src/retroshare/rsjsonapi.h index 5f9ad4d87..7e69531ca 100644 --- a/src/retroshare/rsjsonapi.h +++ b/src/retroshare/rsjsonapi.h @@ -208,10 +208,14 @@ public: * should take care of not using a path range already used by the jsonAPI * server */ + using ResourceProviderSet = std::set< + std::reference_wrapper, + std::less >; + virtual void registerResourceProvider(const JsonApiResourceProvider&) = 0; virtual void unregisterResourceProvider(const JsonApiResourceProvider&) = 0; virtual bool hasResourceProvider(const JsonApiResourceProvider&) = 0; - virtual const std::set,std::less >& getResourceProviders() const =0; + virtual ResourceProviderSet getResourceProviders() const = 0; /** * @brief This function should be used by JSON API clients that aren't diff --git a/src/retroshare/rsplugin.h b/src/retroshare/rsplugin.h index 0939edd5c..4d57dddd2 100644 --- a/src/retroshare/rsplugin.h +++ b/src/retroshare/rsplugin.h @@ -88,8 +88,8 @@ namespace resource_api class StateTokenServer; } -// Plugin API version. Not used yet, but will be in the future the -// main value that decides for compatibility. +// Plugin API version. RsPluginManager rejects plugins built against a +// different version before calling into their ABI. // #define RS_PLUGIN_API_VERSION 0x000102 @@ -123,7 +123,6 @@ public: RsUtil::inited_ptr mDht; RsUtil::inited_ptr mServiceControl; RsUtil::inited_ptr mPluginHandler; - RsUtil::inited_ptr mJsonApi; // gxs std::string mGxsDir; @@ -143,6 +142,9 @@ public: #ifdef RS_USE_WIRE RsUtil::inited_ptr mWire; #endif + + // Keep new members appended so offsets used by older plugins do not move. + RsUtil::inited_ptr mJsonApi; }; class RsPlugin diff --git a/src/rsserver/rsinit.cc b/src/rsserver/rsinit.cc index 711c35fd7..97e35bea8 100644 --- a/src/rsserver/rsinit.cc +++ b/src/rsserver/rsinit.cc @@ -454,9 +454,7 @@ void RsInit::startupWebServices(const RsConfigOptions& conf,bool force_start_jso jas = new JsonApiServer(); rsJsonApi = jas; } - // A plugin provider also requires the server even when the built-in WebUI - // is disabled. Providers were registered during plugin initialization. - bool jsonapi_needed = force_start_jsonapi || !jas->getResourceProviders().empty(); + bool jsonapi_needed = force_start_jsonapi; // add jsonapi server to config manager so that it can save/load its tokens p3ConfigMgr *cfgmgr = dynamic_cast(RsControl::instance()->configManager()); @@ -1710,8 +1708,27 @@ int RsServer::StartupRetroShare() interfaces.mGxsTunnels = mGxsTunnels; interfaces.mReputations = mReputations; interfaces.mPosted = mPosted; - + +#ifdef RS_JSONAPI + // Service and Android start JSON API before plugins are initialized. Take + // one snapshot here so all newly registered providers are published with + // at most one core-owned restart after every plugin received interfaces. + const bool jsonApiWasRunning = rsJsonApi && rsJsonApi->isRunning(); + const std::size_t jsonApiProviderCount = jsonApiWasRunning ? + rsJsonApi->getResourceProviders().size() : 0; +#endif mPluginsManager->setInterfaces(interfaces); +#ifdef RS_JSONAPI + if( jsonApiWasRunning && jsonApiProviderCount != + rsJsonApi->getResourceProviders().size() ) + { + RsInfo() << "Restarting JSON API once to publish plugin resources."; + const std::error_condition restartError = rsJsonApi->restart(true); + if(restartError) + RsErr() << "Failed restarting JSON API after plugin initialization: " + << restartError.message(); + } +#endif // now add plugin objects inside the loop: // - client services provided by plugins. From 2edbb0910e97f9b60bc4ba3f630db3742c7a6107 Mon Sep 17 00:00:00 2001 From: defnax <9952056+defnax@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:21:20 +0200 Subject: [PATCH 3/7] Fix api version --- src/retroshare/rsplugin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/retroshare/rsplugin.h b/src/retroshare/rsplugin.h index 4d57dddd2..708c38569 100644 --- a/src/retroshare/rsplugin.h +++ b/src/retroshare/rsplugin.h @@ -91,7 +91,7 @@ namespace resource_api // Plugin API version. RsPluginManager rejects plugins built against a // different version before calling into their ABI. // -#define RS_PLUGIN_API_VERSION 0x000102 +#define RS_PLUGIN_API_VERSION 0x000103 // Used for the status of plugins. // From 1ad692430447d0279434f03c41b0ad162ee32e32 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 11 Aug 2026 21:56:27 +0200 Subject: [PATCH 4/7] fix(jsonapi): dispatch the post-plugin restart off the API server thread StartupRetroShare() restarts the JSON API once after setInterfaces() so plugin resource providers get published in retroshare-service and on Android, where the server is already running by then. Those are exactly the two setups where StartupRetroShare() can be reached from the JSON API server thread: it is called by RsLoginHelper::attemptLogin() and createLocationV2(), both exposed through the API, and restbed serves its handlers on the thread that called Service::start() (no worker limit is set, so restbed defaults to running the io_context inline) -- that thread being JsonApiServer::run(). restart() calls RsThread::fullstop(). Joining our own thread makes waitWhileStopping() print an error and return without waiting, and the following RsThread::start() then fails with "attempt to start already running thread". The server ends up stopped for good, the in-flight login response is never delivered, and the plugin routes this restart exists to publish are still missing. Wrap the restart in RsThread::async, the same way the /rsJsonApi/restart handler already does for the same reason. As a side effect the RESTART_BURST_PROTECTION wait, which throttles API clients rather than the core, no longer blocks startup. --- src/rsserver/rsinit.cc | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/rsserver/rsinit.cc b/src/rsserver/rsinit.cc index 97e35bea8..7144b7ef5 100644 --- a/src/rsserver/rsinit.cc +++ b/src/rsserver/rsinit.cc @@ -1723,10 +1723,25 @@ int RsServer::StartupRetroShare() rsJsonApi->getResourceProviders().size() ) { RsInfo() << "Restarting JSON API once to publish plugin resources."; - const std::error_condition restartError = rsJsonApi->restart(true); - if(restartError) - RsErr() << "Failed restarting JSON API after plugin initialization: " - << restartError.message(); + + /* The restart must not run on the JSON API server thread. + * StartupRetroShare() is reached from RsLoginHelper::attemptLogin() and + * createLocationV2(), both exposed through the JSON API, and restbed + * serves its handlers on the thread that called Service::start(), which + * is JsonApiServer::run() itself. restart() calls RsThread::fullstop(), + * and joining our own thread makes waitWhileStopping() bail out with an + * error instead of waiting; the subsequent RsThread::start() then fails + * with "attempt to start already running thread" and the server stays + * down for good. The /rsJsonApi/restart handler takes the same care. + * Dispatching also keeps the RESTART_BURST_PROTECTION wait, which is + * meant to throttle API clients, off the startup path. */ + RsThread::async([]() + { + const std::error_condition restartError = rsJsonApi->restart(true); + if(restartError) + RsErr() << "Failed restarting JSON API after plugin " + << "initialization: " << restartError.message(); + }); } #endif From 890480881afefc96cadbf029679008997356bd4f Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 11 Aug 2026 21:57:06 +0200 Subject: [PATCH 5/7] docs(jsonapi): record the invariants the plugin handoff relies on No behaviour change, three comment-level fixes from the review: - rsjsonapi.h: the ResourceProviderSet alias had been inserted between the doc comment and the functions that comment documents, so the comment read as documentation for the alias. Move the alias above it and note that getResourceProviders() now returns a snapshot by value. - rsjsonapi.h: rsJsonApi being non-null no longer implies the server is configured, since StartupRetroShare() now publishes it before startupWebServices() applies the config manager, tokens, port and binding address. Existing code uses the null-ness of that pointer as a phase marker, so say it on the declaration. - rsinit.cc: the creation block has to stay after the pre-existing connectToConfigManager() block, otherwise the GUI connects the config manager twice and reloads jsonapi.cfg twice. That ordering was load bearing and undocumented. --- src/retroshare/rsjsonapi.h | 17 ++++++++++++----- src/rsserver/rsinit.cc | 6 ++++++ 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/retroshare/rsjsonapi.h b/src/retroshare/rsjsonapi.h index 7e69531ca..de1121264 100644 --- a/src/retroshare/rsjsonapi.h +++ b/src/retroshare/rsjsonapi.h @@ -37,6 +37,11 @@ class RsJsonApi; /** * Pointer to global instance of RsJsonApi service implementation * @jsonapi{development} + * + * Beware that non-null does not imply configured: RsServer::StartupRetroShare() + * creates the server early so plugins can register resource providers, while + * the config manager, the authorized tokens, the listening port and the binding + * address are only applied later by RsInit::startupWebServices(). */ extern RsJsonApi* rsJsonApi; @@ -203,15 +208,17 @@ public: */ virtual void connectToConfigManager(p3ConfigMgr& cfgmgr) = 0; - /** - * This is used to add/remove new web services to JsonAPI. The client - * should take care of not using a path range already used by the jsonAPI - * server - */ using ResourceProviderSet = std::set< std::reference_wrapper, std::less >; + /** + * This is used to add/remove new web services to JsonAPI. The client + * should take care of not using a path range already used by the jsonAPI + * server. + * @see getResourceProviders() returns a snapshot by value, so it stays + * valid while another thread registers or unregisters a provider. + */ virtual void registerResourceProvider(const JsonApiResourceProvider&) = 0; virtual void unregisterResourceProvider(const JsonApiResourceProvider&) = 0; virtual bool hasResourceProvider(const JsonApiResourceProvider&) = 0; diff --git a/src/rsserver/rsinit.cc b/src/rsserver/rsinit.cc index 7144b7ef5..11c1e7534 100644 --- a/src/rsserver/rsinit.cc +++ b/src/rsserver/rsinit.cc @@ -1687,6 +1687,12 @@ int RsServer::StartupRetroShare() // The GUI configures web services after plugins are initialized. Create // the server now so plugins can register providers in setInterfaces(). // startupWebServices() will configure and reuse this same instance later. + // + // This has to stay *after* the "if (rsJsonApi) connectToConfigManager()" + // block above: that block exists for retroshare-service and Android, where + // startupWebServices() already ran before login. Creating the server before + // it would make the GUI connect the config manager here and again in + // startupWebServices(), reloading jsonapi.cfg twice. if(!rsJsonApi) rsJsonApi = new JsonApiServer(); interfaces.mJsonApi = rsJsonApi; From bc638f0cb9ccc410b79bd734d0371888f583eb4b Mon Sep 17 00:00:00 2001 From: jolavillette Date: Tue, 11 Aug 2026 22:55:33 +0200 Subject: [PATCH 6/7] fix(shutdown): stop the JSON API before plugins delete their providers rsGlobalShutDown() stopped the JSON API almost last, after stopPlugins(). A plugin that registered a JsonApiResourceProvider deletes it in its stop(), but the running restbed service still holds the restbed::Resource objects that provider returned, and their handlers capture it. Any request served between stopPlugins() and the fullstop at the end of the function therefore dereferences freed memory. The window is not theoretical: everything in between -- UPnP teardown, the auto-proxy shutdown, all registered service threads, the RsServer tick thread and the per-peer streamers -- can take tens of seconds, and a web interface polls throughout. Move the fullstop to the top of the function. It also keeps an API client from touching the configuration after ConfigFinalSave(), and it must stay outside the wasReady branch: retroshare-service and Android start the JSON API before login, so a shutdown from that state has to stop it too. Without this, a plugin has to restart the whole JSON API from its stop() to make deleting its own provider safe, which costs a burst-protection wait and brings the server back up in the middle of teardown. --- src/rsserver/p3face-config.cc | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/rsserver/p3face-config.cc b/src/rsserver/p3face-config.cc index 16a309ca5..6af2767c7 100644 --- a/src/rsserver/p3face-config.cc +++ b/src/rsserver/p3face-config.cc @@ -84,6 +84,21 @@ void RsServer::rsGlobalShutDown() bool wasReady = coreReady; coreReady = false; +#ifdef RS_JSONAPI + /* Stop the JSON API before anything else. Plugins delete their + * JsonApiResourceProvider in stopPlugins() below, while the restbed service + * still holds the resources that provider handed out -- their handlers + * capture it, so serving a request in that window dereferences freed + * memory. The window is not small: everything between stopPlugins() and the + * end of this function can take tens of seconds, and a web interface polls + * throughout. Stopping first also keeps an API client from touching the + * configuration after ConfigFinalSave(). + * + * Not inside the wasReady branch: retroshare-service and Android start the + * JSON API before login, so a shutdown from that state must stop it too. */ + if(rsJsonApi) rsJsonApi->fullstop(); +#endif + if(wasReady) { /* Close the incoming-connection listener FIRST, before anything else. @@ -123,10 +138,6 @@ void RsServer::rsGlobalShutDown() * iterating the peer list concurrently. */ if(pqih) pqih->fullstopAllThreads(); -#ifdef RS_JSONAPI - if(rsJsonApi) rsJsonApi->fullstop(); -#endif - AuthPGP::exit(); // close all databases From 62d05674ba1bdb7437fae8f0cd91b54f00ed5fe0 Mon Sep 17 00:00:00 2001 From: jolavillette Date: Sat, 15 Aug 2026 09:34:16 +0200 Subject: [PATCH 7/7] jsonapi: do not replace rsJsonApi silently, and assign it once startupWebServices() casts rsJsonApi to JsonApiServer to reuse the instance the plugin handoff created. If that cast ever fails, the previous code built a second server and overwrote the global without a word -- while every plugin still holds the pointer it was handed in setInterfaces(), now pointing at an object nobody drives. Unreachable as things stand, since rsJsonApi is only ever set to a JsonApiServer; worth one line of log rather than a silent swap. The trailing `rsJsonApi = jas;` at the end of the function repeated what the same function already did on the line above the cast, and only when it had created the server itself. Dropped. --- src/rsserver/rsinit.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/rsserver/rsinit.cc b/src/rsserver/rsinit.cc index 11c1e7534..dd0cdddb3 100644 --- a/src/rsserver/rsinit.cc +++ b/src/rsserver/rsinit.cc @@ -451,6 +451,15 @@ void RsInit::startupWebServices(const RsConfigOptions& conf,bool force_start_jso JsonApiServer* jas = dynamic_cast(rsJsonApi); if(!jas) { + /* Not reachable today -- rsJsonApi is only ever set to a JsonApiServer, + * here and in the plugin handoff of StartupRetroShare(). Should that + * change, replacing it silently would leave every plugin holding the + * pointer it received in setInterfaces() talking to an abandoned + * object, with no trace of why. */ + if(rsJsonApi) + RsErr() << "rsJsonApi is set but is not a JsonApiServer. Replacing " + << "it: plugins still hold the previous pointer."; + jas = new JsonApiServer(); rsJsonApi = jas; } @@ -537,8 +546,6 @@ void RsInit::startupWebServices(const RsConfigOptions& conf,bool force_start_jso } else RsInfo() << " Not starting JSON API, since it is currently not required by any service." ; - - rsJsonApi = jas; } #endif