Skip to content

Commit

Permalink
feat : handle thread safe issue for getCurrentFutures (#1843)
Browse files Browse the repository at this point in the history
### 📝 Description
improve thread safe issue for getCurrentFutures

current implementation get the values form cachemap for dataloaders.

And make it flatten. But when we get the values it's just iterator, not
all the elements is obtained.

So when we run flatten, it traverse the each iterator, and it make
```ArrayIndexOutOfBoundException```

Following is error message

```
aused by: java.lang.ArrayIndexOutOfBoundsException: Index 279 out of bounds for length 279
	at java.base/java.util.HashMap.valuesToArray(HashMap.java:973)
	at java.base/java.util.HashMap$Values.toArray(HashMap.java:1050)
	at java.base/java.util.ArrayList.addAll(ArrayList.java:670)
	at kotlin.collections.CollectionsKt__MutableCollectionsKt.addAll(MutableCollections.kt:116)
	at kotlin.collections.CollectionsKt__IterablesKt.flatten(Iterables.kt:49)
	at com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistry.getCurrentFutures(KotlinDataLoaderRegistry.kt:67)
	at com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistry.dispatchAll(KotlinDataLoaderRegistry.kt:78)
	at graphql.execution.instrumentation.dataloader.FieldLevelTrackingApproach.dispatch(FieldLevelTrackingApproach.java:242)
	at graphql.execution.instrumentation.dataloader.FieldLevelTrackingApproach$1.onFieldValuesInfo(FieldLevelTrackingApproach.java:150)
	at graphql.execution.AsyncExecutionStrategy.lambda$execute$1(AsyncExecutionStrategy.java:72)
	at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
	... 50 common frames omitted
```

### 🔗 Related Issues
#1837
  • Loading branch information
younseunghyun authored Sep 14, 2023
1 parent 8cc8169 commit 11c24e8
Showing 1 changed file with 5 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@ class KotlinDataLoaderRegistry(
* @return list of current completable futures.
*/
fun getCurrentFutures(): List<CompletableFuture<*>> =
synchronized(registry.dataLoaders) {
registry.dataLoaders.map { dataLoader ->
dataLoader.cacheMap.all
}.flatten()
}
registry.dataLoaders.map { dataLoader ->
synchronized(dataLoader) {
dataLoader.cacheMap.all.toList()
}
}.flatten()

/**
* This will invoke [DataLoader.dispatch] on each of the registered [DataLoader]s,
Expand Down

0 comments on commit 11c24e8

Please sign in to comment.