From a3831b2134903b799334ed30b90b588b3ce08903 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 13 Nov 2018 14:14:05 +0700 Subject: [PATCH] rename removing of connection IDs to retiring --- client.go | 2 +- client_test.go | 4 +- mock_packet_handler_manager_test.go | 12 +++--- mock_session_runner_test.go | 12 +++--- packet_handler_map.go | 8 ++-- packet_handler_map_test.go | 4 +- server.go | 10 ++--- session.go | 2 +- session_test.go | 58 ++++++++++++++--------------- 9 files changed, 56 insertions(+), 56 deletions(-) diff --git a/client.go b/client.go index 55fa89be5..f3bad6a00 100644 --- a/client.go +++ b/client.go @@ -406,7 +406,7 @@ func (c *client) createNewTLSSession(version protocol.VersionNumber) error { defer c.mutex.Unlock() runner := &runner{ onHandshakeCompleteImpl: func(_ Session) { close(c.handshakeChan) }, - removeConnectionIDImpl: c.packetHandlers.Remove, + retireConnectionIDImpl: c.packetHandlers.Retire, } sess, err := newClientSession( c.conn, diff --git a/client_test.go b/client_test.go index ae2f48929..3f65943e8 100644 --- a/client_test.go +++ b/client_test.go @@ -311,7 +311,7 @@ var _ = Describe("Client", func() { It("removes closed sessions from the multiplexer", func() { manager := NewMockPacketHandlerManager(mockCtrl) manager.EXPECT().Add(connID, gomock.Any()) - manager.EXPECT().Remove(connID) + manager.EXPECT().Retire(connID) mockMultiplexer.EXPECT().AddConn(packetConn, gomock.Any()).Return(manager, nil) var runner sessionRunner @@ -334,7 +334,7 @@ var _ = Describe("Client", func() { return sess, nil } sess.EXPECT().run().Do(func() { - runner.removeConnectionID(connID) + runner.retireConnectionID(connID) }) _, err := DialContext( diff --git a/mock_packet_handler_manager_test.go b/mock_packet_handler_manager_test.go index fef4eb847..8d4fd1c95 100644 --- a/mock_packet_handler_manager_test.go +++ b/mock_packet_handler_manager_test.go @@ -54,14 +54,14 @@ func (mr *MockPacketHandlerManagerMockRecorder) CloseServer() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloseServer", reflect.TypeOf((*MockPacketHandlerManager)(nil).CloseServer)) } -// Remove mocks base method -func (m *MockPacketHandlerManager) Remove(arg0 protocol.ConnectionID) { - m.ctrl.Call(m, "Remove", arg0) +// Retire mocks base method +func (m *MockPacketHandlerManager) Retire(arg0 protocol.ConnectionID) { + m.ctrl.Call(m, "Retire", arg0) } -// Remove indicates an expected call of Remove -func (mr *MockPacketHandlerManagerMockRecorder) Remove(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Remove", reflect.TypeOf((*MockPacketHandlerManager)(nil).Remove), arg0) +// Retire indicates an expected call of Retire +func (mr *MockPacketHandlerManagerMockRecorder) Retire(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Retire", reflect.TypeOf((*MockPacketHandlerManager)(nil).Retire), arg0) } // SetServer mocks base method diff --git a/mock_session_runner_test.go b/mock_session_runner_test.go index 4ef433ce8..8bfccf67c 100644 --- a/mock_session_runner_test.go +++ b/mock_session_runner_test.go @@ -44,12 +44,12 @@ func (mr *MockSessionRunnerMockRecorder) onHandshakeComplete(arg0 interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "onHandshakeComplete", reflect.TypeOf((*MockSessionRunner)(nil).onHandshakeComplete), arg0) } -// removeConnectionID mocks base method -func (m *MockSessionRunner) removeConnectionID(arg0 protocol.ConnectionID) { - m.ctrl.Call(m, "removeConnectionID", arg0) +// retireConnectionID mocks base method +func (m *MockSessionRunner) retireConnectionID(arg0 protocol.ConnectionID) { + m.ctrl.Call(m, "retireConnectionID", arg0) } -// removeConnectionID indicates an expected call of removeConnectionID -func (mr *MockSessionRunnerMockRecorder) removeConnectionID(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "removeConnectionID", reflect.TypeOf((*MockSessionRunner)(nil).removeConnectionID), arg0) +// retireConnectionID indicates an expected call of retireConnectionID +func (mr *MockSessionRunnerMockRecorder) retireConnectionID(arg0 interface{}) *gomock.Call { + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "retireConnectionID", reflect.TypeOf((*MockSessionRunner)(nil).retireConnectionID), arg0) } diff --git a/packet_handler_map.go b/packet_handler_map.go index 4f3b69507..ebc751e26 100644 --- a/packet_handler_map.go +++ b/packet_handler_map.go @@ -51,11 +51,11 @@ func (h *packetHandlerMap) Add(id protocol.ConnectionID, handler packetHandler) h.mutex.Unlock() } -func (h *packetHandlerMap) Remove(id protocol.ConnectionID) { - h.removeByConnectionIDAsString(string(id)) +func (h *packetHandlerMap) Retire(id protocol.ConnectionID) { + h.retireByConnectionIDAsString(string(id)) } -func (h *packetHandlerMap) removeByConnectionIDAsString(id string) { +func (h *packetHandlerMap) retireByConnectionIDAsString(id string) { time.AfterFunc(h.deleteClosedSessionsAfter, func() { h.mutex.Lock() delete(h.handlers, id) @@ -79,7 +79,7 @@ func (h *packetHandlerMap) CloseServer() { go func(id string, handler packetHandler) { // session.Close() blocks until the CONNECTION_CLOSE has been sent and the run-loop has stopped _ = handler.Close() - h.removeByConnectionIDAsString(id) + h.retireByConnectionIDAsString(id) wg.Done() }(id, handler) } diff --git a/packet_handler_map_test.go b/packet_handler_map_test.go index bc18c561e..f5ac5320e 100644 --- a/packet_handler_map_test.go +++ b/packet_handler_map_test.go @@ -92,7 +92,7 @@ var _ = Describe("Packet Handler Map", func() { handler.deleteClosedSessionsAfter = 10 * time.Millisecond connID := protocol.ConnectionID{1, 2, 3, 4, 5, 6, 7, 8} handler.Add(connID, NewMockPacketHandler(mockCtrl)) - handler.Remove(connID) + handler.Retire(connID) time.Sleep(30 * time.Millisecond) Expect(handler.handlePacket(nil, getPacket(connID))).To(MatchError("received a packet with an unexpected connection ID 0x0102030405060708")) }) @@ -105,7 +105,7 @@ var _ = Describe("Packet Handler Map", func() { packetHandler.EXPECT().GetPerspective().Return(protocol.PerspectiveClient) packetHandler.EXPECT().handlePacket(gomock.Any()) handler.Add(connID, packetHandler) - handler.Remove(connID) + handler.Retire(connID) err := handler.handlePacket(nil, getPacket(connID)) Expect(err).ToNot(HaveOccurred()) }) diff --git a/server.go b/server.go index cf435e492..20a58a415 100644 --- a/server.go +++ b/server.go @@ -32,8 +32,8 @@ type unknownPacketHandler interface { type packetHandlerManager interface { Add(protocol.ConnectionID, packetHandler) + Retire(protocol.ConnectionID) SetServer(unknownPacketHandler) - Remove(protocol.ConnectionID) CloseServer() } @@ -48,16 +48,16 @@ type quicSession interface { type sessionRunner interface { onHandshakeComplete(Session) - removeConnectionID(protocol.ConnectionID) + retireConnectionID(protocol.ConnectionID) } type runner struct { onHandshakeCompleteImpl func(Session) - removeConnectionIDImpl func(protocol.ConnectionID) + retireConnectionIDImpl func(protocol.ConnectionID) } func (r *runner) onHandshakeComplete(s Session) { r.onHandshakeCompleteImpl(s) } -func (r *runner) removeConnectionID(c protocol.ConnectionID) { r.removeConnectionIDImpl(c) } +func (r *runner) retireConnectionID(c protocol.ConnectionID) { r.retireConnectionIDImpl(c) } var _ sessionRunner = &runner{} @@ -152,7 +152,7 @@ func listen(conn net.PacketConn, tlsConf *tls.Config, config *Config) (*server, func (s *server) setup() error { s.sessionRunner = &runner{ onHandshakeCompleteImpl: func(sess Session) { s.sessionQueue <- sess }, - removeConnectionIDImpl: s.sessionHandler.Remove, + retireConnectionIDImpl: s.sessionHandler.Retire, } cookieGenerator, err := handshake.NewCookieGenerator() if err != nil { diff --git a/session.go b/session.go index 2304455c1..1de985b66 100644 --- a/session.go +++ b/session.go @@ -424,7 +424,7 @@ runLoop: } s.closed.Set(true) s.logger.Infof("Connection %s closed.", s.srcConnID) - s.sessionRunner.removeConnectionID(s.srcConnID) + s.sessionRunner.retireConnectionID(s.srcConnID) s.cryptoStreamHandler.Close() return closeErr.err } diff --git a/session_test.go b/session_test.go index 7ab2bf1b1..d63a447a4 100644 --- a/session_test.go +++ b/session_test.go @@ -326,7 +326,7 @@ var _ = Describe("Session", func() { It("handles CONNECTION_CLOSE frames", func() { testErr := qerr.Error(qerr.ProofInvalid, "foobar") streamManager.EXPECT().CloseWithError(testErr) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() go func() { @@ -367,7 +367,7 @@ var _ = Describe("Session", func() { It("shuts down without error", func() { streamManager.EXPECT().CloseWithError(qerr.Error(qerr.PeerGoingAway, "")) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{raw: []byte("connection close")}, nil) Expect(sess.Close()).To(Succeed()) @@ -379,7 +379,7 @@ var _ = Describe("Session", func() { It("only closes once", func() { streamManager.EXPECT().CloseWithError(qerr.Error(qerr.PeerGoingAway, "")) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) Expect(sess.Close()).To(Succeed()) @@ -392,7 +392,7 @@ var _ = Describe("Session", func() { It("closes streams with proper error", func() { testErr := errors.New("test error") streamManager.EXPECT().CloseWithError(qerr.Error(0x1337, testErr.Error())) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) sess.CloseWithError(0x1337, testErr) @@ -402,7 +402,7 @@ var _ = Describe("Session", func() { It("closes the session in order to replace it with another QUIC version", func() { streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.destroy(errCloseSessionForNewVersion) Eventually(areSessionsRunning).Should(BeFalse()) @@ -411,7 +411,7 @@ var _ = Describe("Session", func() { It("cancels the context when the run loop exists", func() { streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) returned := make(chan struct{}) @@ -429,7 +429,7 @@ var _ = Describe("Session", func() { It("retransmits the CONNECTION_CLOSE packet if packets are arriving late", func() { streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{raw: []byte("foobar")}, nil) sess.Close() @@ -504,7 +504,7 @@ var _ = Describe("Session", func() { Expect(err).To(MatchError(testErr)) close(done) }() - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) sess.handlePacket(&receivedPacket{header: hdr}) Eventually(done).Should(BeClosed()) }) @@ -735,7 +735,7 @@ var _ = Describe("Session", func() { Consistently(mconn.written).Should(HaveLen(2)) // make the go routine return packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() Eventually(done).Should(BeClosed()) @@ -762,7 +762,7 @@ var _ = Describe("Session", func() { Consistently(mconn.written).Should(HaveLen(1)) // make the go routine return packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() Eventually(done).Should(BeClosed()) @@ -791,7 +791,7 @@ var _ = Describe("Session", func() { Eventually(mconn.written, 2*pacingDelay).Should(HaveLen(2)) // make the go routine return packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() Eventually(done).Should(BeClosed()) @@ -817,7 +817,7 @@ var _ = Describe("Session", func() { Eventually(mconn.written).Should(HaveLen(3)) // make the go routine return packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() Eventually(done).Should(BeClosed()) @@ -838,7 +838,7 @@ var _ = Describe("Session", func() { sess.scheduleSending() // no packet will get sent Consistently(mconn.written).ShouldNot(Receive()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() sess.Close() @@ -866,7 +866,7 @@ var _ = Describe("Session", func() { sess.scheduleSending() Eventually(mconn.written).Should(Receive()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() @@ -900,7 +900,7 @@ var _ = Describe("Session", func() { Eventually(mconn.written).Should(Receive()) // make sure the go routine returns packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() @@ -912,7 +912,7 @@ var _ = Describe("Session", func() { It("closes when RunHandshake() errors", func() { testErr := errors.New("crypto setup error") streamManager.EXPECT().CloseWithError(qerr.Error(qerr.InternalError, testErr.Error())) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) go func() { @@ -934,7 +934,7 @@ var _ = Describe("Session", func() { }() Consistently(sess.Context().Done()).ShouldNot(BeClosed()) // make sure the go routine returns - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() @@ -963,7 +963,7 @@ var _ = Describe("Session", func() { Eventually(done).Should(BeClosed()) //make sure the go routine returns streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() Expect(sess.Close()).To(Succeed()) @@ -979,7 +979,7 @@ var _ = Describe("Session", func() { close(done) }() streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() Expect(sess.Close()).To(Succeed()) @@ -997,7 +997,7 @@ var _ = Describe("Session", func() { close(done) }() streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() Expect(sess.CloseWithError(0x1337, testErr)).To(Succeed()) @@ -1021,7 +1021,7 @@ var _ = Describe("Session", func() { sess.processTransportParameters(params) // make the go routine return streamManager.EXPECT().CloseWithError(gomock.Any()) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() sess.Close() @@ -1055,7 +1055,7 @@ var _ = Describe("Session", func() { }() Eventually(sent).Should(BeClosed()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() @@ -1076,7 +1076,7 @@ var _ = Describe("Session", func() { }() Consistently(mconn.written).ShouldNot(Receive()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() @@ -1097,7 +1097,7 @@ var _ = Describe("Session", func() { }() Consistently(mconn.written).ShouldNot(Receive()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) streamManager.EXPECT().CloseWithError(gomock.Any()) packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) cryptoSetup.EXPECT().Close() @@ -1112,7 +1112,7 @@ var _ = Describe("Session", func() { }) It("times out due to no network activity", func() { - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) sess.handshakeComplete = true sess.lastNetworkActivityTime = time.Now().Add(-time.Hour) done := make(chan struct{}) @@ -1133,7 +1133,7 @@ var _ = Describe("Session", func() { It("times out due to non-completed handshake", func() { sess.sessionCreationTime = time.Now().Add(-protocol.DefaultHandshakeTimeout).Add(-time.Second) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) { Expect(f.ErrorCode).To(Equal(qerr.HandshakeTimeout)) @@ -1166,7 +1166,7 @@ var _ = Describe("Session", func() { }() Consistently(sess.Context().Done()).ShouldNot(BeClosed()) // make the go routine return - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() sess.Close() Eventually(sess.Context().Done()).Should(BeClosed()) @@ -1174,7 +1174,7 @@ var _ = Describe("Session", func() { It("closes the session due to the idle timeout after handshake", func() { packer.EXPECT().PackPacket().AnyTimes() - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() packer.EXPECT().PackConnectionClose(gomock.Any()).DoAndReturn(func(f *wire.ConnectionCloseFrame) (*packedPacket, error) { Expect(f.ErrorCode).To(Equal(qerr.NetworkIdleTimeout)) @@ -1348,7 +1348,7 @@ var _ = Describe("Client Session", func() { Expect(err).ToNot(HaveOccurred()) // make sure the go routine returns packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil) - sessionRunner.EXPECT().removeConnectionID(gomock.Any()) + sessionRunner.EXPECT().retireConnectionID(gomock.Any()) cryptoSetup.EXPECT().Close() Expect(sess.Close()).To(Succeed()) Eventually(sess.Context().Done()).Should(BeClosed())