From e4de93e5108ddefeff8d3f9639a1ca950dccbef8 Mon Sep 17 00:00:00 2001
From: Matt Fenelon <matt.fenelon@carwow.co.uk>
Date: Fri, 12 Apr 2024 11:46:33 +0100
Subject: [PATCH] Add a failing test case to show why deadlocks are occurring

This include some logging for diagnostics to show what's happening when
the deadlock occurs.

```
Graphiti::Scope
  #resolve_sideloads
    when the requested sideload exists on the resource
      with concurrency
        with nested sideloads greater than Graphiti.config.concurrency_max_threads
thread 6220: employees queuing positions
thread 6220: employees waiting on [:positions]
thread 6240: running positions
thread 6240: positions queuing department
thread 6240: positions waiting on [:department]
          does not deadlock (FAILED - 1)

Failures:

  1) Graphiti::Scope#resolve_sideloads when the requested sideload exists on the resource with concurrency with nested sideloads greater than Graphiti.config.concurrency_max_threads does not deadlock
     Failure/Error: expect { instance.resolve_sideloads(results) }.not_to raise_error

       expected no Exception, got #<fatal:"No live threads left. Deadlock?\n2 threads, 2 sleeps current:0x00007f7e6f7b1780 main thread:...or.rb:339 sleep_forever>\n   rb_thread_t:0x00007f7e6f7b1780 native:0x000070000cfb4000 int:0\n   \n"> with backtrace:
         # ./lib/graphiti/scope.rb:78:in `resolve_sideloads'
         # ./spec/scope_spec.rb:145:in `block (7 levels) in <top (required)>'
         # ./spec/scope_spec.rb:145:in `block (6 levels) in <top (required)>'
     # ./spec/scope_spec.rb:145:in `block (6 levels) in <top (required)>'
```
---
 lib/graphiti/scope.rb |  6 +++++-
 spec/scope_spec.rb    | 31 ++++++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/lib/graphiti/scope.rb b/lib/graphiti/scope.rb
index f527d8ab..0ca221d2 100644
--- a/lib/graphiti/scope.rb
+++ b/lib/graphiti/scope.rb
@@ -59,12 +59,14 @@ def resolve_sideloads(results)
         parent_resource = @resource
         graphiti_context = Graphiti.context
         resolve_sideload = -> {
+          puts "thread #{Thread.current.object_id}: running #{name}"
           Graphiti.config.before_sideload&.call(graphiti_context)
           Graphiti.context = graphiti_context
           sideload.resolve(results, q, parent_resource)
           @resource.adapter.close if concurrent
         }
         if concurrent
+          puts "thread #{Thread.current.object_id}: #{@resource.class.type} queuing #{name}"
           promises << Concurrent::Promise.execute(executor: self.class.global_thread_pool_executor, &resolve_sideload)
         else
           resolve_sideload.call
@@ -72,7 +74,9 @@ def resolve_sideloads(results)
       end
 
       if concurrent
-        Concurrent::Promise.zip(*promises, executor: self.class.global_thread_pool_executor).value!
+        puts "thread #{Thread.current.object_id}: #{@resource.class.type} waiting on #{@query.sideloads.map(&:first)}"
+        Concurrent::Promise.zip(*promises).value!
+        puts "thread #{Thread.current.object_id}: #{@resource.class.type} finished waiting on #{@query.sideloads.map(&:first)}"
       end
     end
 
diff --git a/spec/scope_spec.rb b/spec/scope_spec.rb
index 84433522..15ee2c20 100644
--- a/spec/scope_spec.rb
+++ b/spec/scope_spec.rb
@@ -75,9 +75,9 @@
   describe "#resolve_sideloads" do
     let(:sideload) { double(shared_remote?: false, name: :positions) }
     let(:results) { [double.as_null_object] }
+    let(:params) { {include: {positions: {}}} }
 
     before do
-      params[:include] = {positions: {}}
       objekt = instance.instance_variable_get(:@object)
       allow(resource).to receive(:resolve).with(objekt) { results }
     end
@@ -119,14 +119,31 @@
           expect { instance.resolve_sideloads(results) }.not_to raise_error
         end
 
-        context 'with more sideloads than the thread pool size' do
-          before { allow(Graphiti.config).to receive(:concurrency_max_threads).and_return(0) }
-
-          it 'deadlocks' do
-            expect { instance.resolve_sideloads(results) }.to raise_error do |e|
-              expect(e.message).to start_with('No live threads left. Deadlock?')
+        context 'with nested sideloads greater than Graphiti.config.concurrency_max_threads' do
+          let(:params) { { include: { positions: { department: {} } } } }
+          let(:position_resource) { PORO::PositionResource.new }
+          let(:departments_sideload) { double(shared_remote?: false, name: :departments) }
+
+          before do
+            stub_const(
+              'Graphiti::Scope::GLOBAL_THREAD_POOL_EXECUTOR',
+              Concurrent::Delay.new {
+                Concurrent::ThreadPoolExecutor.new(max_threads: 1, fallback_policy: :caller_runs)
+              }
+            )
+
+            allow(position_resource.class).to receive(:sideload).with(:department) { departments_sideload }
+            allow(departments_sideload).to receive(:resolve).and_return(departments_sideload)
+
+            # make resolve just load the sideloads
+            allow(sideload).to receive(:resolve) do |results, q, _parent_resource|
+              described_class.new(sideload.as_null_object, position_resource, q).resolve_sideloads(results)
             end
           end
+
+          it 'does not deadlock' do
+            expect { instance.resolve_sideloads(results) }.not_to raise_error
+          end
         end
       end