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 race condition in HystrixThreadPoolKey.asKey method #1271

Merged
merged 2 commits into from Jul 11, 2016

Conversation

daniilguit
Copy link
Contributor

Fix rare case when this method would return different values for the same key

The problem would happen when:

  1. Thread1: No such key when intern.get(name), so k was null
  2. Thread1: We would enter if-statement
  3. Thread2: Other thread would add key to intern map
  4. Thread1: We will not put new key to intern map, but we will return newly created value instead of one stored in map

Fix rare case when this method would return different values for the same key
@chrisgray
Copy link
Contributor

Should we make similar fixes to HystrixCommandGroupKey and HystrixCommandKey?

@daniilguit
Copy link
Contributor Author

daniilguit commented Jul 8, 2016

I think it worth to extract this functionality to new utility class. I can do this also and update pull request

@mattrjacobs
Copy link
Contributor

This looks like a good change to me. Thanks @daniilguit ! Would you like me to merge this or wait until you're done with your refactoring?

@daniilguit
Copy link
Contributor Author

Let's merge the final result, I will update pull request in a few days.

* Extract internation logic to InternMap utility class
* Introduce common HystrixKey interface
@daniilguit
Copy link
Contributor Author

Committed code unification.

@mattrjacobs
Copy link
Contributor

Thanks @daniilguit ! Looks great.

@mattrjacobs mattrjacobs merged commit e27686b into Netflix:master Jul 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants