Skip to content

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

Merged
defnax merged 1 commit into
defnax:api-for-pluginsfrom
jolavillette:fix/jsonapi-shutdown-order-for-364
Aug 14, 2026
Merged

defnax merged 1 commit into
defnax:api-for-pluginsfrom
jolavillette:fix/jsonapi-shutdown-order-for-364

Conversation

@jolavillette

Copy link
Copy Markdown

One commit, already open standalone as RetroShare#366. Proposing it here as well on purpose — see the last section for why the duplication is deliberate.

The defect

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.

Why it belongs in this PR

The defect is only reachable once a plugin can register a resource provider — which is exactly what this PR enables. Before it, no plugin in the tree has access to RsJsonApi, so the code path does not exist.

It also closes a merge-order trap. RetroShare/RetroShare#3289 no longer restarts the JSON API from FeedReaderPlugin::stop(); that restart used to be what made deleting the provider safe, and this change is what replaces it. #3289 already declares a hard dependency on this PR — without it the plugin does not even compile, since RsPlugInInterfaces::mJsonApi would not exist. Carrying the shutdown fix here therefore makes it impossible to merge #3289 without the protection, instead of relying on someone reading a note about merge order.

On the duplication with RetroShare#366

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

Not build-tested. The MINGW64 workflow in this repo fails at "Checkout submodules" on every branch including master (it lists the super-project's submodules, which do not exist here), so its red mark is unrelated.

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.
@defnax

defnax commented Aug 14, 2026

Copy link
Copy Markdown
Owner

this not mergeable i saw it today

@jolavillette

Copy link
Copy Markdown
Author

#6 merges cleanly — it is a fast-forward on top of api-for-plugins, one commit, no conflict. I verified it locally with git merge-tree as well as on GitHub, which reports MERGEABLE.

@defnax
defnax merged commit 2f62c24 into defnax:api-for-plugins Aug 14, 2026
1 check failed
@jolavillette
jolavillette deleted the fix/jsonapi-shutdown-order-for-364 branch August 15, 2026 14:06
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.

2 participants