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

core: document that NameResolver is not thread-safe. #2918

Closed

Conversation

zhangkun83
Copy link
Contributor

Resolves #2649. This makes resolvers easier to implement.

Resolves grpc#2649.  This makes resolvers easier to implement.
}
savedListener = listener;
resolving = true;
// If this task is started by refresh(), there might already be a scheduled task.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if resolutionRunnable runs on a thread other than the original one it ran on. I don't know if ChannelExecutor is guaranteed to be serial.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you are right. ChannelExecutor needs to be exposed to NameResolver so that tasks ran in an executor can be serialized with it. I will put this PR on hold until I have time to improve it.

@zhangkun83
Copy link
Contributor Author

Superseded by #5364

@zhangkun83 zhangkun83 closed this Feb 21, 2019
@zhangkun83 zhangkun83 deleted the nameresolver_nonthreadsafe branch February 21, 2019 23:40
@lock lock bot locked as resolved and limited conversation to collaborators May 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NameResolver can be NotThreadSafe
2 participants