Skip to content

fix(plugin-ext): stop stale plugin host RPC protocols from hijacking replies - #17925

Draft
dr14-make wants to merge 1 commit into
eclipse-theia:masterfrom
dr14-make:fix/stale-plugin-rpc-log-storm
Draft

fix(plugin-ext): stop stale plugin host RPC protocols from hijacking replies#17925
dr14-make wants to merge 1 commit into
eclipse-theia:masterfrom
dr14-make:fix/stale-plugin-rpc-log-storm

Conversation

@dr14-make

@dr14-make dr14-make commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

What it does

Fixes #17924, which has the full mechanism and the prior reports.

After the frontend reconnects, a disposed RPCProtocolImpl keeps answering the plugin host. createServerRpc discarded its HostedPluginWatcher.onPostMessageEvent disposable, and that watcher is a singleton outliving every connection with a constant pluginHostId, so the dead protocol keeps receiving. Emitter dispatch is FIFO, so it answers from an empty locals map before the live protocol does and wins the reply race — every main-side call from the plugin host fails. The plugin host then reports each failure through PluginLogger, whose $log is an RPC request whose rejection was dropped, so the unhandled-rejection reporter logs through the same broken channel and one failed log becomes thousands per second.

  • hosted-plugin.ts — dispose the subscription and close the channel with the protocol, announcing the close first so in-flight requests are rejected rather than left pending.
  • logger.ts — swallow the $log rejection, so a log failure cannot report itself.
  • rpc-protocol.ts — report messages handled after disposal as ConnectionClosedError; an emptied locals map otherwise reads as a missing service registration.

How to test

npm test --workspace @theia/plugin-ext. Two new specs, and I confirmed each fails with its half of the change reverted rather than assuming it:

  • hosted/browser/hosted-plugin-rpc.spec.ts pins the reply race. It wires a plugin host to createServerRpc through a shared watcher, serves one request, disposes that connection, opens a second, and asserts the answer comes from the live protocol. Against the pre-fix createServerRpc it fails with expected 'first connection' to equal 'second connection' — the disposed protocol answering first.
  • common/rpc-protocol.spec.ts covers the post-disposal error, the in-flight rejection on channel close, and the logger no longer leaving an unhandled rejection.

Worth noting from writing that first test: with the subscription disposal alone reverted the test still passes, because closing the channel already stops delivery. The two teardown steps overlap — the subscription disposal is what stops the listener accumulating per reconnect, the channel close is what stops a disposed protocol receiving.

Manually, with the default frontendConnectionTimeout: suspend and wake the machine on an Electron app with plugins active. Before, the log fills with paired no local service handler with id LoggerMain messages at an accelerating rate and the application stops responding; after, they are gone and it stays responsive.

Follow-ups

Recorded in #17924 rather than here: the teardown also leaves the plugin session unusable (the plugin host is re-forked, so extensions lose their state, while main-side registries keep the previous connection's registrations), and it only happens at all because the default frontendConnectionTimeout: 0 makes a successful reconnect impossible. Also unexplained there: a duplicate-reply stream that survives this fix.

Two smaller ones: PluginWorker leaks its worker.onmessage/onerror wiring the same way and never terminates the Worker; and ConnectionClosedError.is() can never match a received error, because msgpackr carries name/message/stack but not the custom code.

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

HostedPluginSupport.initRpc and createServerRpc take an additional toDisconnect: DisposableCollection, matching how the rest of the file passes it around. Both are protected: a subclass that calls them no longer compiles, and one that overrides createServerRpc with the old signature still compiles but silently misses the teardown. The alternative that avoids the break is a composite disposable covering subscription, channel and protocol so the existing toDisconnect.push(rpc) carries all three — happy to switch.

Attribution

Parts of this change and its investigation were produced with AI assistance. I have read and verified every line of the diff, reproduced the bug and the fix myself, and am accountable for the contribution.

Review checklist

Reminder for reviewers

@github-project-automation github-project-automation Bot moved this to Waiting on reviewers in PR Backlog Aug 17, 2026
@dr14-make
dr14-make force-pushed the fix/stale-plugin-rpc-log-storm branch from 13ce107 to e52c4ac Compare August 17, 2026 11:08
@dr14-make
dr14-make marked this pull request as draft August 17, 2026 11:22
@dr14-make
dr14-make force-pushed the fix/stale-plugin-rpc-log-storm branch from e52c4ac to b797a4e Compare August 17, 2026 13:19
…replies

When the frontend connection closes, `toDisconnect.dispose()` disposes the
`RPCProtocolImpl` serving the plugin host, but `createServerRpc` discarded the
`HostedPluginWatcher.onPostMessageEvent` disposable. The watcher is a singleton
that outlives every connection and `pluginHostId` is the same on each of them,
so the subscription survived and kept feeding plugin host messages into the
disposed protocol's channel.

On reconnect, `onDidOpenConnection` re-runs the load and adds a second protocol
and subscription. Emitter dispatch is FIFO, so the disposed protocol saw every
message first and answered it from an empty `locals` map, beating the current
protocol to the reply; the correct reply then arrived second and was dropped as
`No reply handler for reply with id: N`. Every main-side call from the plugin
host therefore failed after the first reconnect.

That failure also compounded. The plugin host routes `console.*` through
`PluginLogger`, whose `$log` is dispatched as an RPC request because its name
starts with neither `notify` nor `on`, and the returned promise was dropped. A
failed log therefore became an unhandled rejection, which the plugin host
reports with two `console.error` calls, each sending another failing `$log`. A
single failed log grew into thousands per second until the application stopped
responding.

Dispose the subscription and close the channel with the protocol, announcing the
close first so requests still in flight are rejected rather than left pending.
Swallow the rejection in `PluginLogger` so a log failure can never report
itself. Report messages handled after disposal as `ConnectionClosedError`, since
an emptied `locals` map otherwise surfaces as a missing service registration and
sends adopters looking for an absent binding.

Signed-off-by: Dmitrij Rozdestvensky <dmitrij.rozdestvensky@juliahub.com>
@dr14-make
dr14-make force-pushed the fix/stale-plugin-rpc-log-storm branch from b797a4e to 3db2d01 Compare August 17, 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

Status: Waiting on reviewers

Development

Successfully merging this pull request may close these issues.

Disposed plugin host RPC protocol hijacks replies after a reconnect, and the plugin host logger amplifies the failure

1 participant