From 4525269cd830ac2ba338d9e1281ea80d66e189cf Mon Sep 17 00:00:00 2001 From: Juan Batiz-Benet Date: Sun, 11 Jan 2015 11:59:25 -0800 Subject: [PATCH] p2p/net/conn/listener: ignore certain errors This should handle early breakages, where a failing connection would take out the listener entirely. There are probably other errors we should be handling here, like secure connection failures. --- p2p/net/conn/dial_test.go | 91 +++++++++++++++++++++++++++++++++++++++ p2p/net/conn/listen.go | 61 +++++++++++++++++++------- 2 files changed, 136 insertions(+), 16 deletions(-) diff --git a/p2p/net/conn/dial_test.go b/p2p/net/conn/dial_test.go index b757a1b69..3c141012c 100644 --- a/p2p/net/conn/dial_test.go +++ b/p2p/net/conn/dial_test.go @@ -1,8 +1,10 @@ package conn import ( + "fmt" "io" "net" + "strings" "testing" "time" @@ -103,6 +105,9 @@ func testDialer(t *testing.T, secure bool) { if !secure { key1 = nil key2 = nil + t.Log("testing insecurely") + } else { + t.Log("testing securely") } ctx, cancel := context.WithCancel(context.Background()) @@ -164,3 +169,89 @@ func TestDialerSecure(t *testing.T) { // t.Skip("Skipping in favor of another test") testDialer(t, true) } + +func testDialerCloseEarly(t *testing.T, secure bool) { + // t.Skip("Skipping in favor of another test") + + p1 := tu.RandPeerNetParamsOrFatal(t) + p2 := tu.RandPeerNetParamsOrFatal(t) + + key1 := p1.PrivKey + if !secure { + key1 = nil + t.Log("testing insecurely") + } else { + t.Log("testing securely") + } + + ctx, cancel := context.WithCancel(context.Background()) + l1, err := Listen(ctx, p1.Addr, p1.ID, key1) + if err != nil { + t.Fatal(err) + } + p1.Addr = l1.Multiaddr() // Addr has been determined by kernel. + + // lol nesting + d2 := &Dialer{ + LocalPeer: p2.ID, + // PrivateKey: key2, -- dont give it key. we'll just close the conn. + } + + errs := make(chan error, 100) + done := make(chan struct{}, 1) + gotclosed := make(chan struct{}, 1) + go func() { + defer func() { done <- struct{}{} }() + + _, err := l1.Accept() + if err != nil { + if strings.Contains(err.Error(), "closed") { + gotclosed <- struct{}{} + return + } + errs <- err + } + errs <- fmt.Errorf("got conn") + }() + + c, err := d2.Dial(ctx, p1.Addr, p1.ID) + if err != nil { + errs <- err + } + c.Close() // close it early. + + readerrs := func() { + for { + select { + case e := <-errs: + t.Error(e) + default: + return + } + } + } + readerrs() + + l1.Close() + <-done + cancel() + readerrs() + close(errs) + + select { + case <-gotclosed: + default: + t.Error("did not get closed") + } +} + +// we dont do a handshake with singleConn, so cant "close early." +// func TestDialerCloseEarlyInsecure(t *testing.T) { +// // t.Skip("Skipping in favor of another test") +// testDialerCloseEarly(t, false) +// } + +func TestDialerCloseEarlySecure(t *testing.T) { + // t.Skip("Skipping in favor of another test") + testDialerCloseEarly(t, true) +} diff --git a/p2p/net/conn/listen.go b/p2p/net/conn/listen.go index 0cc1a75c8..c2049eacc 100644 --- a/p2p/net/conn/listen.go +++ b/p2p/net/conn/listen.go @@ -2,13 +2,14 @@ package conn import ( "fmt" + "io" "net" context "github.com/jbenet/go-ipfs/Godeps/_workspace/src/code.google.com/p/go.net/context" ctxgroup "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-ctxgroup" ma "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr" manet "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr-net" - + tec "github.com/jbenet/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-temp-err-catcher" ic "github.com/jbenet/go-ipfs/p2p/crypto" peer "github.com/jbenet/go-ipfs/p2p/peer" ) @@ -46,25 +47,53 @@ func (l *listener) Accept() (net.Conn, error) { // Contexts and io don't mix. ctx := context.Background() - maconn, err := l.Listener.Accept() - if err != nil { - return nil, err + var catcher tec.TempErrCatcher + + catcher.IsTemp = func(e error) bool { + // ignore connection breakages up to this point. but log them + if e == io.EOF { + log.Debugf("listener ignoring conn with EOF: %s", e) + return true + } + + te, ok := e.(tec.Temporary) + if ok { + log.Debugf("listener ignoring conn with temporary err: %s", e) + return te.Temporary() + } + return false } - c, err := newSingleConn(ctx, l.local, "", maconn) - if err != nil { - return nil, fmt.Errorf("Error accepting connection: %v", err) - } + for { + maconn, err := l.Listener.Accept() + if err != nil { + if catcher.IsTemporary(err) { + continue + } + return nil, err + } - if l.privk == nil { - log.Warning("listener %s listening INSECURELY!", l) - return c, nil + c, err := newSingleConn(ctx, l.local, "", maconn) + if err != nil { + if catcher.IsTemporary(err) { + continue + } + return nil, err + } + + if l.privk == nil { + log.Warning("listener %s listening INSECURELY!", l) + return c, nil + } + sc, err := newSecureConn(ctx, l.privk, c) + if err != nil { + if catcher.IsTemporary(err) { + continue + } + return nil, err + } + return sc, nil } - sc, err := newSecureConn(ctx, l.privk, c) - if err != nil { - return nil, fmt.Errorf("Error securing connection: %v", err) - } - return sc, nil } func (l *listener) Addr() net.Addr {