From b42bad8481bcd2bdb9ada2d4845aa9d24421cf97 Mon Sep 17 00:00:00 2001 From: Lucas Clemente Date: Mon, 9 May 2016 17:50:13 +0200 Subject: [PATCH] fix flaky server tests ref #65 --- server.go | 4 +- server_test.go | 135 ++++++++++++++++++++++++------------------------ session.go | 4 +- session_test.go | 10 ++-- 4 files changed, 76 insertions(+), 77 deletions(-) diff --git a/server.go b/server.go index 667c61ae..0e3e0666 100644 --- a/server.go +++ b/server.go @@ -129,9 +129,9 @@ func (s *Server) handlePacket(conn *net.UDPConn, remoteAddr *net.UDPAddr, packet return nil } -func (s *Server) closeCallback(session *Session) { +func (s *Server) closeCallback(id protocol.ConnectionID) { s.sessionsMutex.Lock() - s.sessions[session.connectionID] = nil + s.sessions[id] = nil s.sessionsMutex.Unlock() } diff --git a/server_test.go b/server_test.go index 426fbdbf..71034ef7 100644 --- a/server_test.go +++ b/server_test.go @@ -70,90 +70,89 @@ var _ = Describe("Server", func() { Expect(server.sessions[0x4cfa9f9b668619f6].(*mockSession).connectionID).To(Equal(protocol.ConnectionID(0x4cfa9f9b668619f6))) Expect(server.sessions[0x4cfa9f9b668619f6].(*mockSession).packetCount).To(Equal(2)) }) + + It("closes and deletes sessions", func() { + pheader := []byte{0x09, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0x51, 0x30, 0x33, 0x32, 0x01} + err := server.handlePacket(nil, nil, append(pheader, (&crypto.NullAEAD{}).Seal(0, pheader, nil)...)) + Expect(err).ToNot(HaveOccurred()) + Expect(server.sessions).To(HaveLen(1)) + server.closeCallback(0x4cfa9f9b668619f6) + // The server should now have closed the session, leaving a nil value in the sessions map + Expect(server.sessions).To(HaveLen(1)) + Expect(server.sessions[0x4cfa9f9b668619f6]).To(BeNil()) + }) + }) - PIt("setups and responds with version negotiation", func() { - server, err := NewServer(testdata.GetTLSConfig(), nil) - Expect(err).ToNot(HaveOccurred()) - go func() { - defer GinkgoRecover() - err := server.ListenAndServe("127.0.0.1:13370") - Expect(err).To(HaveOccurred()) - }() - - time.Sleep(50 * time.Millisecond) - addr, err2 := net.ResolveUDPAddr("udp", "127.0.0.1:13370") - Expect(err2).ToNot(HaveOccurred()) - conn, err2 := net.DialUDP("udp", nil, addr) - Expect(err2).ToNot(HaveOccurred()) - _, err2 = conn.Write([]byte{0x09, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x01, 'Q', '0', '0', '0', 0x01}) - Expect(err2).ToNot(HaveOccurred()) - data := make([]byte, 1000) - n, _, err2 := conn.ReadFromUDP(data) - Expect(err2).ToNot(HaveOccurred()) - data = data[:n] - expected := append( - []byte{0x3d, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0}, - protocol.SupportedVersionsAsTags..., - ) - Expect(data).To(Equal(expected)) - err2 = server.Close() - Expect(err2).ToNot(HaveOccurred()) - }) - - PIt("setups and responds with error on invalid frame", func() { - server, err := NewServer(testdata.GetTLSConfig(), nil) - Expect(err).ToNot(HaveOccurred()) - go func() { - defer GinkgoRecover() - err := server.ListenAndServe("127.0.0.1:13370") - Expect(err).To(HaveOccurred()) - }() - - time.Sleep(50 * time.Millisecond) - addr, err2 := net.ResolveUDPAddr("udp", "127.0.0.1:13370") - Expect(err2).ToNot(HaveOccurred()) - conn, err2 := net.DialUDP("udp", nil, addr) - Expect(err2).ToNot(HaveOccurred()) - _, err2 = conn.Write([]byte{0x09, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x01, 'Q', '0', '0', '0', 0x01, 0x00}) - Expect(err2).ToNot(HaveOccurred()) - data := make([]byte, 1000) - n, _, err2 := conn.ReadFromUDP(data) - Expect(err2).ToNot(HaveOccurred()) - Expect(n).ToNot(BeZero()) - time.Sleep(20 * time.Millisecond) - err2 = server.Close() - Expect(err2).ToNot(HaveOccurred()) - }) - - PIt("closes and deletes sessions", func() { + It("setups and responds with version negotiation", func(done Done) { server, err := NewServer(testdata.GetTLSConfig(), nil) Expect(err).ToNot(HaveOccurred()) go func() { defer GinkgoRecover() err := server.ListenAndServe("127.0.0.1:13370") Expect(err).To(HaveOccurred()) + close(done) }() addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:13370") Expect(err).ToNot(HaveOccurred()) - - // Send an invalid packet - time.Sleep(50 * time.Millisecond) conn, err := net.DialUDP("udp", nil, addr) Expect(err).ToNot(HaveOccurred()) - pheader := []byte{0x09, 0xf6, 0x19, 0x86, 0x66, 0x9b, 0x9f, 0xfa, 0x4c, 0x51, 0x30, 0x33, 0x32, 0x01} - _, err = conn.Write(append(pheader, (&crypto.NullAEAD{}).Seal(0, pheader, nil)...)) - Expect(err).ToNot(HaveOccurred()) - time.Sleep(10 * time.Millisecond) - - // The server should now have closed the session, leaving a nil value in the sessions map - Expect(server.sessions).To(HaveLen(1)) - // Expect(server.sessions[0x4cfa9f9b668619f6]).To(BeNil()) - Expect(server.sessions[0x4cfa9f9b668619f6]).To(BeNil()) + Eventually(func() error { + _, err = conn.Write([]byte{0x09, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x01, 'Q', '0', '0', '0', 0x01}) + if err != nil { + return err + } + data := make([]byte, 1000) + n, _, err := conn.ReadFromUDP(data) + if err != nil { + return err + } + data = data[:n] + expected := append( + []byte{0x3d, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + protocol.SupportedVersionsAsTags..., + ) + Expect(data).To(Equal(expected)) + return nil + }).ShouldNot(HaveOccurred()) err = server.Close() Expect(err).ToNot(HaveOccurred()) - }) + }, 1) + + It("setups and responds with error on invalid frame", func(done Done) { + server, err := NewServer(testdata.GetTLSConfig(), nil) + Expect(err).ToNot(HaveOccurred()) + go func() { + defer GinkgoRecover() + err := server.ListenAndServe("127.0.0.1:13370") + Expect(err).To(HaveOccurred()) + close(done) + }() + + addr, err := net.ResolveUDPAddr("udp", "127.0.0.1:13370") + Expect(err).ToNot(HaveOccurred()) + conn, err := net.DialUDP("udp", nil, addr) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() error { + _, err = conn.Write([]byte{0x09, 0x01, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x01, 'Q', '0', '0', '0', 0x01, 0x00}) + if err != nil { + return err + } + data := make([]byte, 1000) + n, _, err := conn.ReadFromUDP(data) + if err != nil { + return err + } + Expect(n).ToNot(BeZero()) + return nil + }).ShouldNot(HaveOccurred()) + + time.Sleep(20 * time.Millisecond) + err = server.Close() + Expect(err).ToNot(HaveOccurred()) + }, 1) }) diff --git a/session.go b/session.go index a32919ea..83928698 100644 --- a/session.go +++ b/session.go @@ -31,7 +31,7 @@ var ( type StreamCallback func(*Session, utils.Stream) // CloseCallback is called when a session is closed -type CloseCallback func(*Session) +type CloseCallback func(id protocol.ConnectionID) // A Session is a QUIC session type Session struct { @@ -313,7 +313,7 @@ func (s *Session) Close(e error, sendConnectionClose bool) error { s.closed = true s.closeChan <- struct{}{} - s.closeCallback(s) + s.closeCallback(s.connectionID) if !sendConnectionClose { return nil diff --git a/session_test.go b/session_test.go index 167da5ed..3d689870 100644 --- a/session_test.go +++ b/session_test.go @@ -94,7 +94,7 @@ var _ = Describe("Session", func() { streamCallback: func(*Session, utils.Stream) { callbackCalled = true }, connectionParametersManager: handshake.NewConnectionParamatersManager(), closeChan: make(chan struct{}, 1), - closeCallback: func(*Session) {}, + closeCallback: func(protocol.ConnectionID) {}, packer: &packetPacker{aead: &crypto.NullAEAD{}}, } }) @@ -261,7 +261,7 @@ var _ = Describe("Session", func() { signer, err := crypto.NewRSASigner(testdata.GetTLSConfig()) Expect(err).ToNot(HaveOccurred()) scfg := handshake.NewServerConfig(crypto.NewCurve25519KEX(), signer) - session = NewSession(conn, 0, 0, scfg, nil, func(*Session) { closed = true }).(*Session) + session = NewSession(conn, 0, 0, scfg, nil, func(protocol.ConnectionID) { closed = true }).(*Session) go session.Run() Expect(runtime.NumGoroutine()).To(Equal(nGoRoutinesBefore + 2)) }) @@ -332,7 +332,7 @@ var _ = Describe("Session", func() { signer, err := crypto.NewRSASigner(testdata.GetTLSConfig()) Expect(err).ToNot(HaveOccurred()) scfg := handshake.NewServerConfig(crypto.NewCurve25519KEX(), signer) - session = NewSession(conn, 0, 0, scfg, nil, func(*Session) {}).(*Session) + session = NewSession(conn, 0, 0, scfg, nil, func(protocol.ConnectionID) {}).(*Session) }) It("sends after queuing a stream frame", func() { @@ -363,7 +363,7 @@ var _ = Describe("Session", func() { signer, err := crypto.NewRSASigner(testdata.GetTLSConfig()) Expect(err).ToNot(HaveOccurred()) scfg := handshake.NewServerConfig(crypto.NewCurve25519KEX(), signer) - session = NewSession(conn, 0, 0, scfg, nil, func(*Session) {}).(*Session) + session = NewSession(conn, 0, 0, scfg, nil, func(protocol.ConnectionID) {}).(*Session) s, err := session.NewStream(3) Expect(err).NotTo(HaveOccurred()) err = session.handleStreamFrame(&frames.StreamFrame{ @@ -415,7 +415,7 @@ var _ = Describe("Session", func() { signer, err := crypto.NewRSASigner(testdata.GetTLSConfig()) Expect(err).ToNot(HaveOccurred()) scfg := handshake.NewServerConfig(crypto.NewCurve25519KEX(), signer) - session = NewSession(conn, 0, 0, scfg, nil, func(*Session) {}).(*Session) + session = NewSession(conn, 0, 0, scfg, nil, func(protocol.ConnectionID) {}).(*Session) cong = &mockCongestion{} session.congestion = cong