Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

race can lead to multiple concurrent connection attempts to same target #1478

Closed
rade opened this issue Sep 29, 2015 · 0 comments
Closed

race can lead to multiple concurrent connection attempts to same target #1478

rade opened this issue Sep 29, 2015 · 0 comments
Labels
Milestone

Comments

@rade
Copy link
Member

rade commented Sep 29, 2015

Apply

diff --git a/router/connection.go b/router/connection.go
index e3967ec..810c419 100644
--- a/router/connection.go
+++ b/router/connection.go
@@ -360,6 +360,7 @@ func (conn *LocalConnection) shutdown(err error) {
                conn.forwarder.Stop()
        }

+       time.Sleep(1 * time.Minute)
        conn.Router.ConnectionMaker.ConnectionTerminated(conn.remoteTCPAddr, err
 }

and then

host1$ weave launch
host2$ weave launch host1
host1$ weave stop
host2$ weave status connections
-> 192.168.48.11:6783    failed      dial tcp4 192.168.48.11:6783: connection refused, retry: 2015-09-29 13:53:41.831462406 +0000 UTC
host2$ sleep 60
host2$ weave status connections
-> 192.168.48.11:6783    failed      read tcp4 192.168.48.11:6783: connection reset by peer, retry: 2015-09-29 13:55:09.21609408 +0000 UTC

This shows an incorrect change of the reported error - the "connection reset by peer" comes from the original failure when we stopped the other peer and stomps on the subsequent "connection refused" error.

However, with slightly different timing, namely the ConnectionTerminated occurring when there is an ongoing connection attempt, we can get multiple concurrent connection attempts to the same target. This breaks a pretty important invariant of the ConnectionMaker, so there may be other bad effects.

@rade rade added the bug label Sep 29, 2015
@rade rade added this to the 1.1.1 milestone Sep 29, 2015
rade added a commit that referenced this issue Sep 29, 2015
rade added a commit that referenced this issue Sep 30, 2015
Get the ConnectionMaker to maintain its own knowledge of the local
connections, based on events emitted in the connection lifecycle.

Fixes #1478.
rade added a commit that referenced this issue Sep 30, 2015
Get the ConnectionMaker to maintain its own knowledge of the local
connections, based on events emitted in the connection lifecycle.

Fixes #1478.
@rade rade modified the milestones: 1.2.0, 1.1.1 Sep 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant