Skip to content

Stop the JSON API before plugins delete their resource providers - #366

Open
jolavillette wants to merge 1 commit into
RetroShare:masterfrom
jolavillette:fix/jsonapi-stop-api-before-plugins
Open

Stop the JSON API before plugins delete their resource providers#366
jolavillette wants to merge 1 commit into
RetroShare:masterfrom
jolavillette:fix/jsonapi-stop-api-before-plugins

Conversation

@jolavillette

Copy link
Copy Markdown
Contributor

RsServer::rsGlobalShutDown() stops 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 — JsonApiServer::run() publishes them once at thread start and the service keeps them for its whole lifetime. 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.

This moves the fullstop() to the top of the function. Two details worth noting:

  • it also keeps an API client from touching the configuration after ConfigFinalSave(), which runs a couple of lines below;
  • it stays outside the wasReady branch, because retroshare-service and Android start the JSON API before login, so a shutdown from that state has to stop it too. That is why the call cannot simply be hoisted into the existing block.

No plugin in the tree registers a resource provider today, so nothing is currently affected. It becomes reachable with #364 and RetroShare/RetroShare#3289, which give plugins access to the JSON API — and without this change, a plugin has to restart the whole JSON API from its stop() just to make deleting its own provider safe, which costs a RESTART_BURST_PROTECTION wait and brings the server back up in the middle of the core teardown.

Sending it separately since it stands on its own and fixes a latent defect for any future provider-registering plugin.

Not build-tested yet.

@jolavillette

Copy link
Copy Markdown
Contributor Author

Heads-up on a deliberate duplication: this same commit is now also proposed on defnax's api-for-plugins branch, i.e. inside #364 (defnax#6).

The reason is merge ordering. RetroShare/RetroShare#3289 no longer restarts the JSON API from FeedReaderPlugin::stop() — that restart was what made deleting the provider safe, and this commit is what replaces it. #3289 already has a hard dependency on #364 (without it the plugin does not compile, RsPlugInInterfaces::mJsonApi would not exist), so carrying the shutdown fix in #364 makes it impossible to merge #3289 without the protection, rather than relying on someone noticing a note about ordering.

This PR stays open as a fallback, so the fix can still reach master on its own if #364 is delayed or dropped. Whichever lands first, the other becomes a no-op: the patch is identical, so git merges it without conflict. I will close this one once #364 is in.

Unrelated to the change: the red mark comes from the MINGW64 workflow, which fails at "Checkout submodules" on every branch including master — .github/workflows/ci-mingw64.yml asks for libbitdht/, libretroshare/ and retroshare-webui/, the super-project's submodules, which do not exist in this repository, so the job dies before CI-Build ever runs. Happy to send a separate PR to fix or drop that workflow if it is wanted.

@jolavillette
jolavillette force-pushed the fix/jsonapi-stop-api-before-plugins branch 2 times, most recently from 3c29d9a to 46fec64 Compare August 13, 2026 22:27
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.
@jolavillette
jolavillette force-pushed the fix/jsonapi-stop-api-before-plugins branch from 46fec64 to 1c6fa59 Compare August 16, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant