Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Networking fixes #480

Merged
merged 4 commits into from Feb 21, 2016
Merged

Networking fixes #480

merged 4 commits into from Feb 21, 2016

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Feb 20, 2016

This PR introduces "expired" state for handshakes and sessions. Since all request are processed asynchronously it was possible that a newly created connection receives an event for a previously deleted or about to be deleted connection with the same id. This resulted in some connections being dropped prematurely.

@arkpar arkpar added the A0-pleasereview 🤓 Pull request needs code review. label Feb 20, 2016
@@ -1 +1 @@
Subproject commit 3116f85a499ceaf4dfdc46726060fc056e2d7829
Subproject commit f32954b3ddb5af2dc3dc9ec6d9a28bee848fdf70
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated test to latest accidentally. They pass anyway

@arkpar arkpar closed this Feb 20, 2016
@arkpar arkpar reopened this Feb 20, 2016
@arkpar arkpar added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 20, 2016
@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Feb 20, 2016
@gavofyork
Copy link
Contributor

needs merge.

@@ -175,13 +175,25 @@ impl Connection {
self.socket.peer_addr()
}

pub fn try_clone(&self) -> io::Result<Self> {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing docs?

@gavofyork
Copy link
Contributor

self.expired() in general seems a bit racey. does it matter?

@gavofyork gavofyork added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 21, 2016
@arkpar
Copy link
Collaborator Author

arkpar commented Feb 21, 2016

It is always checked with an active lock. It was introduced to make sure that all connection events are ignored from the point when it is decided that connection is no longer needed (kill_connection) to the moment it is deregistered and deleted.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 21, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 21, 2016
gavofyork pushed a commit that referenced this pull request Feb 21, 2016
@gavofyork gavofyork merged commit 7512961 into master Feb 21, 2016
@gavofyork gavofyork deleted the network branch February 21, 2016 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants