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] Fixed problem with handling the lingering links case in non-blocking group reader #1835

Closed

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Mar 1, 2021

The problem is described in #1805, though may need further testing.

This PR also contains a fix for lock ordering at the handshake, might need to be moved.

@ethouris ethouris requested a review from maxsharabayko March 1, 2021 09:25
@ethouris ethouris marked this pull request as draft March 1, 2021 09:25
@ethouris ethouris changed the title [core] Fixed problem with elephant case in non-blocking group reader [core] Fixed problem with handling the lingering links case in non-blocking group reader Mar 1, 2021
@mbakholdina mbakholdina added this to the v1.4.3 milestone Mar 1, 2021
@ethouris
Copy link
Collaborator Author

This remains as draft due to enclosing two different solutions. The fix for wrong lock on m_ControlLock was necessary for proper testing the problem. The fix for wrong epoll update has been tested by the issue reporter.

@maxsharabayko maxsharabayko modified the milestones: v1.4.3, v1.4.4 Apr 13, 2021
@gou4shi1
Copy link
Contributor

why not merge this pr?

@ethouris
Copy link
Collaborator Author

I remerged this with latest upstream; the fix for the deadlock on ConnectionLock is already merged. The remaining thing is the avoidance of the spurious read-ready for group.

@ethouris
Copy link
Collaborator Author

Note that I have doubts about the provided solution mainly because this newly introduced flag seems to be required to be filled from a separate thread, so likely this field should be atomic. There's another PR that provides atomic and should be cleared first.

@gou4shi1
Copy link
Contributor

gou4shi1 commented Jun 3, 2021

Great, I found your atomic PR is merged, so this PR is unblocked now?

The remaining thing is the avoidance of the spurious read-ready for group.

The remaining part I care is just if (ready_sockets.empty())) throw CUDTException(); which should be a hotfix, LOL

@maxsharabayko
Copy link
Collaborator

Closed by #2203

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.

4 participants