Skip to content
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.

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

Fix silent thread leak for chained instrumentation #2818

Merged
merged 4 commits into from May 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
@@ -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