p2p/net/swarm: dial once at a time #549
Conversation
jbenet
force-pushed
the
fix-simultaneous-dials
branch
from
eac07a6
to
90d654d
Compare
|
@briantigerchow RFCR? |
| // may have received an incoming connection! if so, we no longer must dial. | ||
| // | ||
| // dial attempts. we may be doing the dialing. if not, we wait. | ||
| attempts := 3 |
|
@jbenet LGTM |
|
|
||
| // ok, we have been charged to dial! let's do it. | ||
| conn, err = s.dial(ctx, p) | ||
| s.dsync.Unlock(p) |
There was a problem hiding this comment.
It wasn't immediately clear that the conn is added to the swarm's list of conns in the dial method, so I thought this lock might've been released too early.
However, I read through and saw that swarm add conn occurs in dialConnSetup.
(nested state changes are somewhat hard to follow. It's nice to have lots of pure code (and to isolate code containing the side-effects, and to keep the side-effects as close to the surface as possible))
but the code looks correct. LGTM
There was a problem hiding this comment.
Agreed, i was annoyed by this, and other warts. it isn't easy to lift up, without carrying a lot of logic into this part, which I wanted to keep very light-- only focused on the sync attempts (turns out dialing is hard!). I added a note 8e888b1 which may clear it up for the reader.
|
ok, thanks! I'll merge when the build greens |
p2p/net/swarm: dial once at a time
This PR makes swarm dials wait for each other.