fix concurrent problem in dispatching strategy

andimarek · andimarek · commit 1d5d4f6b0691 · 2025-06-14T21:38:32.000+10:00
improve test
diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java
@@ -513,7 +513,10 @@ private void dispatchDLCFImpl(Set<String> resultPathsToDispatch, @Nullable Integ
 
         // filter out all DataLoaderCFS that are matching the fields we want to dispatch
         List<ResultPathWithDataLoader> relevantResultPathWithDataLoader = new ArrayList<>();
-        for (ResultPathWithDataLoader resultPathWithDataLoader : callStack.allResultPathWithDataLoader) {
+        // we need to copy the list because the callStack.allResultPathWithDataLoader can be modified concurrently
+        // while iterating over it
+        ArrayList<ResultPathWithDataLoader> resultPathWithDataLoaders = new ArrayList<>(callStack.allResultPathWithDataLoader);
+        for (ResultPathWithDataLoader resultPathWithDataLoader : resultPathWithDataLoaders) {
             if (resultPathsToDispatch.contains(resultPathWithDataLoader.resultPath)) {
                 relevantResultPathWithDataLoader.add(resultPathWithDataLoader);
             }
diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/Issue1178DataLoaderDispatchTest.groovy
@@ -21,9 +21,8 @@ import static graphql.schema.idl.TypeRuntimeWiring.newTypeWiring
 
 class Issue1178DataLoaderDispatchTest extends Specification {
 
-    public static final int NUM_OF_REPS = 100
 
-    @RepeatUntilFailure(maxAttempts = 20)
+    @RepeatUntilFailure(maxAttempts = 100)
     def "shouldn't dispatch twice in multithreaded env"() {
         setup:
         def sdl = """
@@ -81,11 +80,10 @@ class Issue1178DataLoaderDispatchTest extends Specification {
                 .build()
 
         then: "execution shouldn't error"
-        for (int i = 0; i < NUM_OF_REPS; i++) {
-            def ei = ExecutionInput.newExecutionInput()
-                    .graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): enableDataLoaderChaining])
-                    .dataLoaderRegistry(dataLoaderRegistry)
-                    .query("""
+        def ei = ExecutionInput.newExecutionInput()
+                .graphQLContext([(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING): enableDataLoaderChaining])
+                .dataLoaderRegistry(dataLoaderRegistry)
+                .query("""
                 query { 
                     getTodos { __typename id 
                         related { id __typename 
@@ -118,10 +116,9 @@ class Issue1178DataLoaderDispatchTest extends Specification {
                         }
                     } 
                 }""").build()
-            def resultCF = graphql.executeAsync(ei)
-            Awaitility.await().until { resultCF.isDone() }
-            assert resultCF.get().errors.empty
-        }
+        def resultCF = graphql.executeAsync(ei)
+        Awaitility.await().until { resultCF.isDone() }
+        assert resultCF.get().errors.empty
         where:
         enableDataLoaderChaining << [true, false]
 
