Skip to content
This repository was archived by the owner on Sep 11, 2025. It is now read-only.

Conversation

@mattjohnsonpint
Copy link
Contributor

Previously, if an SSE connection was open and streaming subscriptions, we'd get a hung deadlock until the connection was closed, after reloading the wasm module (such as if user code changes were deployed, or in a modus dev loop).

The main deadlock was related to the map in our "dynamic mux". I replaced it with a lock-free xsync map to resolve that issue.

I also found that an open subscription was preventing clean shutdown, which was resolve by keeping track of open subscriptions and cancelling them during shutdown.

Also includes a few other misc bugs that were noticed along the way.

@mattjohnsonpint mattjohnsonpint requested review from a team and Copilot June 12, 2025 02:18
@linear
Copy link

linear bot commented Jun 12, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a deadlock issue related to open inbound HTTP connections by replacing the locking mechanism in the dynamic mux with a lock-free xsync map, and by canceling open GraphQL subscriptions during shutdown.

  • Replace the sync-based route handling with xsync.Map in the HTTP dynamic mux.
  • Introduce cancellation for GraphQL subscriptions on shutdown.
  • Minor refactor in the actor event publishing for clarity.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
runtime/httpserver/server.go Registers a shutdown callback to cancel GraphQL subscriptions.
runtime/httpserver/dynamicMux.go Replaces sync map with xsync.Map and updates route replacement.
runtime/graphql/graphql.go Adds cancellation support for GraphQL subscription contexts.
runtime/actors/agents.go Refactors error handling in getActorPid usage.
CHANGELOG.md Updates the changelog to document the fix.
Comments suppressed due to low confidence (2)

runtime/httpserver/dynamicMux.go:71

  • [nitpick] Consider renaming the variable 'temp' to 'newRoutes' to more clearly indicate its purpose in the ReplaceRoutes function.
temp := maps.Clone(routes)

runtime/httpserver/dynamicMux.go:53

  • [nitpick] Consider renaming the variable 'found' to 'handled' to better convey that it flags whether a matching route was handled during the Range iteration.
var found bool

@mattjohnsonpint mattjohnsonpint enabled auto-merge (squash) June 12, 2025 02:22
@mattjohnsonpint mattjohnsonpint merged commit dfe5648 into main Jun 12, 2025
33 checks passed
@mattjohnsonpint mattjohnsonpint deleted the mjp/hyp-3483-open-subscriptions-are-blocking-some-runtime-operations branch June 12, 2025 02:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants