FFFF Fix silent thread leak for chained instrumentation by KammererTob · Pull Request #2818 · graphql-java/graphql-java · GitHub
[go: up one dir, main page]
Skip to content

Fix silent thread leak for chained instrumentation #2818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bbakerman merged 4 commits into from
May 17, 2022
Merged

Fix silent thread leak for chained instrumentation #2818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
be6eeff
fix for #2068
KammererTob May 23, 2021 10BC0
a1b2965
Adjusted test as per comment
KammererTob May 24, 2021
818529b
Merge remote-tracking branch 'upstream/master'
KammererTob May 11, 2022
da72a41
Fix silent thread leak for chained instrumentation
KammererTob May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ public void onFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList) {
contexts.forEach(context -> context.onFieldValuesInfo(fieldValueInfoList));
}

@Override
public void onFieldValuesException() {
contexts.forEach(ExecutionStrategyInstrumentationContext::onFieldValuesException);
}
}

}
Expand Down
37 changes: 36 additions & 1 deletion src/test/groovy/graphql/Issue2068.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql

import graphql.execution.instrumentation.ChainedInstrumentation
import graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation
import graphql.schema.DataFetcher
import graphql.schema.DataFetchingEnvironment
Expand All @@ -11,6 +12,7 @@ import org.dataloader.DataLoader
import org.dataloader.DataLoaderOptions
import org.dataloader.DataLoaderRegistry
import spock.lang.Specification
import spock.lang.Timeout

import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionStage
Expand Down Expand Up @@ -119,7 +121,40 @@ class Issue2068 extends Specification {
""")
.build())

then: "execution shouldn't hang"
then: "execution with single instrumentation shouldn't hang"
// wait for each future to complete and grab the results
thrown(RuntimeException)

when:
graphql = GraphQL.newGraphQL(schema)
.instrumentation(new ChainedInstrumentation(
Collections.singletonList(new DataLoaderDispatcherInstrumentation()))
)
.build()

graphql.execute(newExecutionInput()
.dataLoaderRegistry(dataLoaderRegistry)
.query("""
query LoadPets {
pets {
cats {
toys {
name
}
}
dogs {
owner {
nation {
name
}
}
}
}
}
""")
.build())

then: "execution with chained instrumentation shouldn't hang"
// wait for each future to complete and grab the results
thrown(RuntimeException)
}
Expand Down
0