[pro] Rails @defer setup can deadlock with development code reloading

[masterful]

Describe the bug

The current GraphQL Pro @defer server setup guide recommends using ActionController::Live to stream multipart responses from Rails.

In development, an ActionController::Live request can deadlock with Rails' code reloader. The problem is in the Rails integration rather than GraphQL execution itself, but following the documented setup exposes GraphQL controllers to it. Because ActionController::Live wraps the whole controller action,
non-deferred requests handled by the same controller are exposed too.

I have opened rails/rails#58486 with a proposed fix. Until a Rails fix lands, could the GraphQL-Ruby docs warn Rails users about this problem or recommend a streaming implementation that does not require ActionController::Live? I would also appreciate feedback on whether the Rails fix preserves the behavior GraphQL-Ruby expects from a streaming controller.

Versions

graphql version: 2.6.7
graphql-pro version: 1.30.2
graphql-enterprise version: 1.7.0
graphql-batch version: 0.6.1
rails: 8.1.3.1
ruby: 4.0.5

GraphQL schema / query

No particular schema/query is required. The deadlock occurs before the Live child thread can run the controller action and commit the response. Any request handled by a controller that includes ActionController::Live can enter the lock cycle.

Steps to reproduce

It's not technically an issue with this gem, but if you're curious, you can reproduce the behaviour by leveraging ActionController::Live...

Instructions here if you're interested

A minimal reproduction is available at:

https://github.com/GetJobber/reproduce-rails-deadlock-behaviour

Set it up with:

 git clone https://github.com/GetJobber/reproduce-rails-deadlock-behaviour.git
 cd reproduce-rails-deadlock-behaviour
 bundle install

Start Rails in development:

 bundle exec rails server -p 3011 -e development

In another terminal, run:

 PORT=3011 ./script/repro.sh

The script performs this sequence:

1. Start an ActionController::Live request.
2. Touch the controller file to request a reload.
3. Start two more Live requests.
4. Report request timeouts and capture /rails/locks.

The reproduction adds a short delay before the Live child reaches the running interlock. This makes the scheduling race deterministic. Thread-pool scheduling, pool pressure, GVL contention, or other work before the child acquires its lock can expose the same window without the artificial delay.

Expected behavior

The streaming controller should complete without deadlocking when Rails detects a code change.

If ActionController::Live cannot safely support development reloading, the GraphQL-Ruby guide should document that limitation and, if possible, recommend another way to serve deferred multipart responses?

Actual behavior

The Rails server can become permanently stuck. Requests time out, and even stopping the development server may become difficult.

Additional context

The Rails tracking issue is rails/rails#56888.

My current proposed fix is rails/rails#58486. It adds a one-shot running continuation for work scheduled by an execution context that already holds a running share.

Under that approach:

• The Live child may claim its continuation ahead of an unload that queued after its parent request was admitted.
• Unrelated new requests remain blocked behind the queued unload.
• A continuation cannot bypass an unload that already holds the exclusive lock.
• Once claimed, the child holds an ordinary running share for the complete action.
• The unload therefore cannot replace constants underneath an active controller action.

An earlier approach in rails/rails#56942 allowed the unload to overtake the parent while it waited for the child. That broke the deadlock, but testing showed that a controller instance created before the reload could begin executing after its constants had been replaced. The continuation approach avoids that mixed-generation behavior.

[rmosolgo]

Hey, thanks for hunting down the fix for this issue and reporting the problem here. Did you find a monkeypatch to unblock this in development? Or is waiting for that Rails fix the only way to unstick it right now?

I'm game to add a note in the docs about this, just curious if there's a local work-around for now.

[masterful]

Monkeypatch is going to be the next step for us if I don't get engagement on the Rails side, yeah.
Don't love doing that for something like ShareLock and Live so I might go with the cheaper fix from the earlier PR that isn't as clean...

(I'm guessing this means that ActionController::Live is the only game in town if we want @defer support?)

[rmosolgo]

the only game in town?

🤔 This is worth considering again. When I first developed @defer, ActionController::Live was the only way of streaming arbitrary data inside the Rails request/response cycle. There might be another way now -- I haven't looked in a long time. I'm going to spend some time investigating and I'll follow up here.

There are two other kinds of possibilities which I haven't seriously investigated because the don't seem practical:

1. Out-of-band @defer updates. For example, @defer payloads come over ActionCable or Pusher instead of in the initial HTTP response. This leaves it to the client to match those payloads with the original query, and introduces the difficulty of enqueuing the work in the initial controller action and the difficulty of resuming the work in a background job (this is now possible with GraphQL::Query::Partial). This seems possible but awfully tricky.
2. Sending queries (or at least @defer queries) over another transport layer, like ActionCable, which supports streaming payloads in another way. Again, seems possible, but also seems error-prone and difficult.

Anyways, I'm going to take some time to see if I can find any more recent work on streaming arbitrary data in Rails and follow up here.

[rmosolgo]

Sadly I'm not seeing a lot out there for other options, just this: https://www.codeotaku.com/journal/2024-11/streaming-rack/index (The "What about Rails" section).

It would work like this:

# app/controllers/graphql_controller.rb
# ... 
def execute 
  result = MySchema.execute(...)
  if (deferrals = res.context[:defer])
    body = ->(stream) { 
      deferrals.each { |deferral|
        stream << "JSON.dump(deferral.to_h)}\n\n"
      end 
    }
    self.response = Rack::Response[200, {"content-type" => "text/event-stream"}, body]
  else 
    render json: result.to_h 
  end 
end 

But this option is pretty minimally documented in Rails (https://api.rubyonrails.org/classes/ActionController/Metal.html#method-i-response-3D).

[masterful]

Yeah that was my fear as well, thank you for looking into this more ❤️
I think one of the concerns (also shared on a different Live issue) is that not splitting this out to its own thread locks up the server thread (the whole point of the Live bit ...)

I don't think I have the appetite to look at out-of-band/alternate-transport updates; although it would theoretically work, it adds a bunch more complexity to the client code as well...

🤔 given this issue isn't with the gem itself, and mostly just looking for alternates, I'd be fine with closing the issue here? I'll float the idea of a monkey patch internally for now if the appetite for @defer is particularly strong on our side.

[rmosolgo]

Continuing to brainstorm... given that the problem is Development-only and related to code reloading, is there any way to eagerly-load code for development requests which include @defer? I'm thinking something like

if Rails.env.development? && params[:query].include?("@defer")
  Zeitwerk::Loader.eager_load_all
end 

In reading the issue and replication script, it sounds like the controller action itself would be "too late" to run that code because you're already in the circumstance where reloading will cause a deadlock (right?).

But what about a Rack middleware, added only in development? (Have I understood the nature of this lock correctly?)

[masterful]

Good question -- it's possible that I've misunderstood how the interlock behaves, but my understanding is that it's not specific to a class that the current controller has loaded specifically, but rather a more broad interlock on class loading in general. So an eager load for a given request doesn't really help us too much?

I had a collapsed section on my PR that visualizes this a bit in case it's helpful:

P = original Live request thread
C = Live child thread
U = request performing the unload

P: Live request              C: Live child               U: Reload request
---------------              -------------               -----------------

acquire running Share

schedule C
                            scheduled, but has
                            not run yet

wait for response commit

                                                         acquire running Share

                                                         request Exclusive
                                                         unload lock

                                                         yield its own
                                                         running Share

                                                         wait for P to
                                                         release its Share

                            request running Share

                            blocked behind U by
                            writer preference

leading to:

P waits for C to commit the response
C waits for U's unload
U waits for P to release its running Share

The reload request doesn't have to be a Graphql controller request -- it just wants the interlock to perform class unload/load. Because the docs suggest to include the ActionController::Live in the GraphQL controller, the response execution will go down the P + C route for each request regardless of whether the @defer is present ... And I don't think you're arguing that we conditionally include the Live piece?

---

It's very possible I've misunderstood your comment though -- big apologies if so. My gut says that eager loading or conditional middleware will not help us here, though?