Skip to content

changed default CoroutineScope to not use GlobalScope to avoid potential cancellation issues and other pitfalls #359

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

Merged
merged 1 commit into from
Jan 30, 2020

Conversation

bsara
Copy link
Contributor

@bsara bsara commented Jan 24, 2020

From the Kotlin docs we read the following (emphasis added):

When a coroutine is launched in the CoroutineScope of another coroutine, it inherits its context via CoroutineScope.coroutineContext and the Job of the new coroutine becomes a child of the parent coroutine's job. When the parent coroutine is cancelled, all its children are recursively cancelled, too.

However, when GlobalScope is used to launch a coroutine, there is no parent for the job of the new coroutine. It is therefore not tied to the scope it was launched from and operates independently.

In order to avoid creating threads that aren't tied to any parent scopes that may exist, GlobalScope has been replaced in this PR with just a normal CoroutineScope that uses the Dispatchers.Default context when no scope is provided via the DataFetchingEnvironment context (I.E. this normal CoroutineScope is the default scope).

@oliemansm oliemansm added this to the 6.0.0 milestone Jan 30, 2020
@oliemansm oliemansm merged commit 5ae378a into graphql-java-kickstart:master Jan 30, 2020
@rrva
Copy link
Contributor

rrva commented Mar 13, 2020

I see that even the old code allowed you to configure the coroutine scope by storing it in the context. How do you do this for all resolvers pre this change (5.7.1)? @oliemansm @bsara

@bsara
Copy link
Contributor Author

bsara commented Apr 1, 2020

@rrva I just used the SchemaParserOptions in my Spring Boot app and had a bean for the options:

typealias CoroutineContextSelector = () -> CoroutineContext

@Configuration
class GraphQLConfig {

  @Bean
  fun coroutineContextSelector(): CoroutineContextSelector = { Dispatchers.DEFAULT }

  @Bean
  fun schemaParserOptions(coroutineContextSelector: CoroutineContextSelectort) = SchemaParserOptions
    .newOptions()
    .coroutineContextProvider(CoroutineContextProvider { coroutineContextSelector() }
    .build()
}

I do the same thing even after updating to 6.0.0, but I don't call build() at the end. If you're not using Spring Boot, you could do the same thing, just make sure that the same options are used when you set everything up.

@bsara bsara deleted the remove-globalscope branch April 1, 2020 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants