Synced with Upstream #24
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 4680cb8 (caused shyiko#321).
Merge conflicts have been resolved. Refactoring and latch related changes have been added. Authentication on greeting packet related code have been retained the same. Signed-off-by: Jude Frederic <[email protected]>
|
hey thanks for the PR. so it looks like the fix for the keepalive race has been really caught up in quite a lot of refactoring. I'm not saying it's bad refactoring, it just makes this PR hard to review and sort of a big code churn that I'd prefer not to take unless I really had to. At this point do you feel like you understand where the deadlock happens well enough that you and I could re-code a fix for it? Alternatively, if you felt like you understood the commits in this PR well enough, if you opened a series of PRs where first the refactors happened and then the actual changes, that'd be ok too. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Upstream repository has the fix for a deadlock issue (shyiko#321). This impacts our project and hence we need this fix and hence so the others. I have resolved the merge conflicts with the upstream project after going over the difference in the changes.
I have retained the greeting packet authentication as of before and nothing has changed with the authentication as it was before. There were conflicts in the first place because of many minor method signature changes which were conflicting with the authentication-related code. Apart from that, all the commits which have been missed from the upstream repository have been resolved of every merge conflict.