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 #1910 protect initPool from being called in parallel #1911

Merged
merged 2 commits into from
Dec 25, 2018
Merged

Conversation

gkorland
Copy link
Contributor

No description provided.

@sazzad16
Copy link
Collaborator

@mp911de
Copy link
Contributor

mp911de commented Dec 12, 2018

I think there are two issues:

  1. Order of invocation
  2. Synchronization/visibility issues
  • initPool(…) in JedisSentinelPool should be called before starting any threads
  • The pool should not be set by any thread but only-re-initialized to avoid visibility issues.

@gkorland
Copy link
Contributor Author

@sazzad16
Copy link
Collaborator

LGTM!

@sazzad16 sazzad16 added this to the 3.0.1 milestone Dec 18, 2018
@gkorland
Copy link
Contributor Author

@mp911de

  1. Order of invocation
    You are right, I guess a cleaner solution should be released later on, I think we should release a quick fix for 3.0.1 and then work on a cleaner solution for 3.1.0
  1. Synchronization/visibility issues
    See commit ed2ed51

@sazzad16
Copy link
Collaborator

@gkorland Isn't before and after of ed2ed51 equivalent?

@gkorland
Copy link
Contributor Author

@sazzad16 no, the former used this as the object to lock on which means it "exposed" the lock object while the later is using a private internal object

@gkorland
Copy link
Contributor Author

@marcosnils @sazzad16 it seems like this bug is too critical and a fix should be released as soon as possible

@sazzad16
Copy link
Collaborator

@gkorland okay. go ahead

@gkorland gkorland merged commit 6126d30 into master Dec 25, 2018
@gkorland gkorland deleted the initPool branch December 25, 2018 08:25
@gkorland gkorland modified the milestones: 3.0.1, 2.10.1 Dec 25, 2018
gkorland added a commit that referenced this pull request Dec 25, 2018
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.

None yet

3 participants