There were multiple synchronization issues in the disconnect handling, all caused by the odd special-casing of Peer.readLoop errors. Remove the special handling of read errors and make readLoop part of the Peer WaitGroup. Thanks to @Gustav-Simonsson for pointing at arrows in a diagram and playing rubber-duck.
		
			
				
	
	
		
			252 lines
		
	
	
		
			6.0 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
			
		
		
	
	
			252 lines
		
	
	
		
			6.0 KiB
		
	
	
	
		
			Go
		
	
	
	
	
	
| package p2p
 | |
| 
 | |
| import (
 | |
| 	"bytes"
 | |
| 	"errors"
 | |
| 	"fmt"
 | |
| 	"math/rand"
 | |
| 	"net"
 | |
| 	"reflect"
 | |
| 	"testing"
 | |
| 	"time"
 | |
| )
 | |
| 
 | |
| var discard = Protocol{
 | |
| 	Name:   "discard",
 | |
| 	Length: 1,
 | |
| 	Run: func(p *Peer, rw MsgReadWriter) error {
 | |
| 		for {
 | |
| 			msg, err := rw.ReadMsg()
 | |
| 			if err != nil {
 | |
| 				return err
 | |
| 			}
 | |
| 			fmt.Printf("discarding %d\n", msg.Code)
 | |
| 			if err = msg.Discard(); err != nil {
 | |
| 				return err
 | |
| 			}
 | |
| 		}
 | |
| 	},
 | |
| }
 | |
| 
 | |
| func testPeer(protos []Protocol) (func(), *conn, *Peer, <-chan DiscReason) {
 | |
| 	fd1, _ := net.Pipe()
 | |
| 	hs1 := &protoHandshake{ID: randomID(), Version: baseProtocolVersion}
 | |
| 	hs2 := &protoHandshake{ID: randomID(), Version: baseProtocolVersion}
 | |
| 	for _, p := range protos {
 | |
| 		hs1.Caps = append(hs1.Caps, p.cap())
 | |
| 		hs2.Caps = append(hs2.Caps, p.cap())
 | |
| 	}
 | |
| 
 | |
| 	p1, p2 := MsgPipe()
 | |
| 	peer := newPeer(fd1, &conn{p1, hs1}, protos)
 | |
| 	errc := make(chan DiscReason, 1)
 | |
| 	go func() { errc <- peer.run() }()
 | |
| 
 | |
| 	closer := func() {
 | |
| 		p1.Close()
 | |
| 		fd1.Close()
 | |
| 	}
 | |
| 	return closer, &conn{p2, hs2}, peer, errc
 | |
| }
 | |
| 
 | |
| func TestPeerProtoReadMsg(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 
 | |
| 	done := make(chan struct{})
 | |
| 	proto := Protocol{
 | |
| 		Name:   "a",
 | |
| 		Length: 5,
 | |
| 		Run: func(peer *Peer, rw MsgReadWriter) error {
 | |
| 			if err := ExpectMsg(rw, 2, []uint{1}); err != nil {
 | |
| 				t.Error(err)
 | |
| 			}
 | |
| 			if err := ExpectMsg(rw, 3, []uint{2}); err != nil {
 | |
| 				t.Error(err)
 | |
| 			}
 | |
| 			if err := ExpectMsg(rw, 4, []uint{3}); err != nil {
 | |
| 				t.Error(err)
 | |
| 			}
 | |
| 			close(done)
 | |
| 			return nil
 | |
| 		},
 | |
| 	}
 | |
| 
 | |
| 	closer, rw, _, errc := testPeer([]Protocol{proto})
 | |
| 	defer closer()
 | |
| 
 | |
| 	Send(rw, baseProtocolLength+2, []uint{1})
 | |
| 	Send(rw, baseProtocolLength+3, []uint{2})
 | |
| 	Send(rw, baseProtocolLength+4, []uint{3})
 | |
| 
 | |
| 	select {
 | |
| 	case <-done:
 | |
| 	case err := <-errc:
 | |
| 		t.Errorf("peer returned: %v", err)
 | |
| 	case <-time.After(2 * time.Second):
 | |
| 		t.Errorf("receive timeout")
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestPeerProtoEncodeMsg(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 
 | |
| 	proto := Protocol{
 | |
| 		Name:   "a",
 | |
| 		Length: 2,
 | |
| 		Run: func(peer *Peer, rw MsgReadWriter) error {
 | |
| 			if err := SendItems(rw, 2); err == nil {
 | |
| 				t.Error("expected error for out-of-range msg code, got nil")
 | |
| 			}
 | |
| 			if err := SendItems(rw, 1, "foo", "bar"); err != nil {
 | |
| 				t.Errorf("write error: %v", err)
 | |
| 			}
 | |
| 			return nil
 | |
| 		},
 | |
| 	}
 | |
| 	closer, rw, _, _ := testPeer([]Protocol{proto})
 | |
| 	defer closer()
 | |
| 
 | |
| 	if err := ExpectMsg(rw, 17, []string{"foo", "bar"}); err != nil {
 | |
| 		t.Error(err)
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestPeerWriteForBroadcast(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 
 | |
| 	closer, rw, peer, peerErr := testPeer([]Protocol{discard})
 | |
| 	defer closer()
 | |
| 
 | |
| 	emptymsg := func(code uint64) Msg {
 | |
| 		return Msg{Code: code, Size: 0, Payload: bytes.NewReader(nil)}
 | |
| 	}
 | |
| 
 | |
| 	// test write errors
 | |
| 	if err := peer.writeProtoMsg("b", emptymsg(3)); err == nil {
 | |
| 		t.Errorf("expected error for unknown protocol, got nil")
 | |
| 	}
 | |
| 	if err := peer.writeProtoMsg("discard", emptymsg(8)); err == nil {
 | |
| 		t.Errorf("expected error for out-of-range msg code, got nil")
 | |
| 	} else if perr, ok := err.(*peerError); !ok || perr.Code != errInvalidMsgCode {
 | |
| 		t.Errorf("wrong error for out-of-range msg code, got %#v", err)
 | |
| 	}
 | |
| 
 | |
| 	// setup for reading the message on the other end
 | |
| 	read := make(chan struct{})
 | |
| 	go func() {
 | |
| 		if err := ExpectMsg(rw, 16, nil); err != nil {
 | |
| 			t.Error(err)
 | |
| 		}
 | |
| 		close(read)
 | |
| 	}()
 | |
| 
 | |
| 	// test successful write
 | |
| 	if err := peer.writeProtoMsg("discard", emptymsg(0)); err != nil {
 | |
| 		t.Errorf("expect no error for known protocol: %v", err)
 | |
| 	}
 | |
| 	select {
 | |
| 	case <-read:
 | |
| 	case err := <-peerErr:
 | |
| 		t.Fatalf("peer stopped: %v", err)
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestPeerPing(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 
 | |
| 	closer, rw, _, _ := testPeer(nil)
 | |
| 	defer closer()
 | |
| 	if err := SendItems(rw, pingMsg); err != nil {
 | |
| 		t.Fatal(err)
 | |
| 	}
 | |
| 	if err := ExpectMsg(rw, pongMsg, nil); err != nil {
 | |
| 		t.Error(err)
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestPeerDisconnect(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 
 | |
| 	closer, rw, _, disc := testPeer(nil)
 | |
| 	defer closer()
 | |
| 	if err := SendItems(rw, discMsg, DiscQuitting); err != nil {
 | |
| 		t.Fatal(err)
 | |
| 	}
 | |
| 	if err := ExpectMsg(rw, discMsg, []interface{}{DiscRequested}); err != nil {
 | |
| 		t.Error(err)
 | |
| 	}
 | |
| 	closer()
 | |
| 	if reason := <-disc; reason != DiscRequested {
 | |
| 		t.Errorf("run returned wrong reason: got %v, want %v", reason, DiscRequested)
 | |
| 	}
 | |
| }
 | |
| 
 | |
| // This test is supposed to verify that Peer can reliably handle
 | |
| // multiple causes of disconnection occurring at the same time.
 | |
| func TestPeerDisconnectRace(t *testing.T) {
 | |
| 	defer testlog(t).detach()
 | |
| 	maybe := func() bool { return rand.Intn(1) == 1 }
 | |
| 
 | |
| 	for i := 0; i < 1000; i++ {
 | |
| 		protoclose := make(chan error)
 | |
| 		protodisc := make(chan DiscReason)
 | |
| 		closer, rw, p, disc := testPeer([]Protocol{
 | |
| 			{
 | |
| 				Name:   "closereq",
 | |
| 				Run:    func(p *Peer, rw MsgReadWriter) error { return <-protoclose },
 | |
| 				Length: 1,
 | |
| 			},
 | |
| 			{
 | |
| 				Name:   "disconnect",
 | |
| 				Run:    func(p *Peer, rw MsgReadWriter) error { p.Disconnect(<-protodisc); return nil },
 | |
| 				Length: 1,
 | |
| 			},
 | |
| 		})
 | |
| 
 | |
| 		// Simulate incoming messages.
 | |
| 		go SendItems(rw, baseProtocolLength+1)
 | |
| 		go SendItems(rw, baseProtocolLength+2)
 | |
| 		// Close the network connection.
 | |
| 		go closer()
 | |
| 		// Make protocol "closereq" return.
 | |
| 		protoclose <- errors.New("protocol closed")
 | |
| 		// Make protocol "disconnect" call peer.Disconnect
 | |
| 		protodisc <- DiscAlreadyConnected
 | |
| 		// In some cases, simulate something else calling peer.Disconnect.
 | |
| 		if maybe() {
 | |
| 			go p.Disconnect(DiscInvalidIdentity)
 | |
| 		}
 | |
| 		// In some cases, simulate remote requesting a disconnect.
 | |
| 		if maybe() {
 | |
| 			go SendItems(rw, discMsg, DiscQuitting)
 | |
| 		}
 | |
| 
 | |
| 		select {
 | |
| 		case <-disc:
 | |
| 		case <-time.After(2 * time.Second):
 | |
| 			// Peer.run should return quickly. If it doesn't the Peer
 | |
| 			// goroutines are probably deadlocked. Call panic in order to
 | |
| 			// show the stacks.
 | |
| 			panic("Peer.run took to long to return.")
 | |
| 		}
 | |
| 	}
 | |
| }
 | |
| 
 | |
| func TestNewPeer(t *testing.T) {
 | |
| 	name := "nodename"
 | |
| 	caps := []Cap{{"foo", 2}, {"bar", 3}}
 | |
| 	id := randomID()
 | |
| 	p := NewPeer(id, name, caps)
 | |
| 	if p.ID() != id {
 | |
| 		t.Errorf("ID mismatch: got %v, expected %v", p.ID(), id)
 | |
| 	}
 | |
| 	if p.Name() != name {
 | |
| 		t.Errorf("Name mismatch: got %v, expected %v", p.Name(), name)
 | |
| 	}
 | |
| 	if !reflect.DeepEqual(p.Caps(), caps) {
 | |
| 		t.Errorf("Caps mismatch: got %v, expected %v", p.Caps(), caps)
 | |
| 	}
 | |
| 
 | |
| 	p.Disconnect(DiscAlreadyConnected) // Should not hang
 | |
| }
 |