Skip to content

Ensure hibernated websockets handle code updates gracefully - #6948

Open
jqmmes wants to merge 2 commits into
mainfrom
joaquim/hibernated-ws-survive-code-updates
Open

Ensure hibernated websockets handle code updates gracefully#6948
jqmmes wants to merge 2 commits into
mainfrom
joaquim/hibernated-ws-survive-code-updates

Conversation

@jqmmes

@jqmmes jqmmes commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

This PR adds the necessary methods to ensure hibernated websockets can survive code updates while hibernated.

jqmmes added 2 commits August 10, 2026 16:34
This commit adds the necessary methods to ensure hibernated websockets can survive code updates
@jqmmes
jqmmes force-pushed the joaquim/hibernated-ws-survive-code-updates branch from e4d85d2 to 2818290 Compare August 10, 2026 15:34
@ask-bonk

ask-bonk Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@jqmmes
jqmmes marked this pull request as ready for review August 11, 2026 11:54
@jqmmes
jqmmes requested review from a team as code owners August 11, 2026 11:54
@jqmmes jqmmes self-assigned this Aug 11, 2026
: typeId(typeId),
params(kj::mv(params)),
manager(manager) {}
manager(manager.addRef()) {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC because of this strong ref:

  1. The hibernation manager owns readLoopTasks.
  2. A read-loop task dispatches a custom event and waits for it to settle.
  3. The custom event strongly owns the hibernation manager.
  4. The manager cannot be destroyed because the event owns it.
  5. The task cannot be cancelled by the manager’s destructor because destruction cannot begin.

If the outer RPC or dispatch never settles, the cycle can remain indefinitely I think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that correct. I'll change the strong ref


namespace {
// Maps each hibernatable WebSocket event ID currently being delivered to the manager that owns the
// socket. The registry is process-global because delivery can cross event loops, but the manager,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it need to be process-global, considering use of the manager requires it to be on the original thread? So there's mutexes + cleanups but can the same not be achieved with thread-local stores?

As I don't think there's a capability in the case of a thread mismatch to run on the original thread, so this could be simplified quite a bit I think if that's the case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it can be simplified.

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