Deadlock in AsyncDataloader with latest async versions

[iyotov-havelock]

Describe the bug Deadlock occurs when executing GraphQL queries with association loading using AsyncDataloader with the async gem after v2.28.1: Relevant changes: https://github.com/socketry/async/compare/v2.28.1...v2.29.0 Likely key change: https://github.com/socketry/async/commit/cf7fce100077552470ceaf32d6a903aa13c0ee30

Versions graphql 2.5.14 Roda 3.89.0 Falcon 0.52.4 async 2.34.0 sequel 5.98.0

GraphQL schema

class Gql::Types::Foo < GraphQL::Schema::Object
  field :id, ID, null: false
  field :name, String, null: false
  field :bar_models, [Gql::Types::Bar], null: false

  def bar_models
    dataloader.with(Sources::RecordsByForeignKey, ::BarModel, :foo_model_id).load(object.id)
  end
end

class Gql::Types::Bar < GraphQL::Schema::Object
  field :id, ID, null: false
  field :name, String, null: false
  field :foo_model, Gql::Types::Foo, null: false

  def foo_model
    dataloader.with(Sources::RecordById, ::FooModel).load(object.foo_model_id)
  end
end

class AppSchema < GraphQL::Schema
  query Gql::Types::Query
  use GraphQL::Dataloader::AsyncDataloader
end

GraphQL query

{
  getAllFoos {
    id
    name
    barModels { id name }
  }
  getAllBars {
    id
    name
    fooModel { id name }
  }
}

(no response - server deadlocks)

Steps to reproduce

Clone this demo repo and run it using the steps provided in the README: https://github.com/iyotov-havelock/async-dataloader-issue

Expected behavior

Query returns all FooModels with their associated BarModels, and all BarModels with their associated FooModel. That's the case when using GraphQL::Dataloader, but not when using GraphQL::Dataloader::AsyncDataloader

Actual behavior

Server deadlocks during dataloader execution

fatal: No live threads left. Deadlock?
1 threads, 1 sleeps current:0x00005c4ab8f61890 main thread:0x00005c4ab8f61890
* #<Thread:0x000071ad9736a6d0 sleep_forever>

→ /gems/async-2.34.0/lib/async/promise.rb:89 in 'Thread::ConditionVariable#wait'
  /gems/async-2.34.0/lib/async/promise.rb:89 in 'block in Async::Promise#wait'
  /gems/async-2.34.0/lib/async/task.rb:258 in 'Async::Task#wait'
  /gems/async-2.34.0/lib/kernel/sync.rb:28 in 'Kernel#Sync'
  /gems/graphql-2.5.14/lib/graphql/dataloader/async_dataloader.rb:34 in 'block in GraphQL::Dataloader::AsyncDataloader#run'

Additional context This looks like a compatibility issue between AsyncDataloader and async gem versions after v2.28.1 Relevant changes: https://github.com/socketry/async/compare/v2.28.1...v2.29.0 Likely key change: https://github.com/socketry/async/commit/cf7fce100077552470ceaf32d6a903aa13c0ee30

[rmosolgo]

Hey, thanks for reporting this and sorry for the trouble! I'll take a close look soon and see how GraphQL-Ruby can mitigate this issue.

Thanks for sharing the reproduction. If you have a chance to copy it into the GraphQL-Ruby test suite, that'd be a good first step. Otherwise I'll give it a shot next time I get a chance.

[rmosolgo]

👋 I got a chance to run your reproduction and it locked up locally just like you said. I reviewed the diffs on Async and looked through the AsyncDataloader code and came up with nothing.

As a random shot in the dark, the top-level spawn_fiber { ... } call caught my eye. I thought maybe Async wouldn't be happy if it was running inside a non-Async-managed fiber. So I swapped it for a Sync { ... } call instead (same effect in this case). It fixed the deadlock, and after one more update, passed the AsyncDataloader tests.

I opened a PR with that change: #5479. Could you try that branch in your app with the new version of async and let me know how it goes? 🤞