mirror of
https://github.com/refraction-networking/uquic.git
synced 2025-04-06 05:37:36 +03:00
refactor how sessions are deleted
Replacing sessions with different structs representing a closed session doesn't work if a session is using multiple connection IDs.
This commit is contained in:
Marten Seemann
parent
9e6bff0b98
commit
03483d5e71
13 changed files with 165 additions and 179 deletions
|
|
@ -7,8 +7,6 @@ import (
|
|||
"crypto/tls"
|
||||
"errors"
|
||||
"net"
|
||||
"runtime/pprof"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
. "github.com/onsi/ginkgo"
|
||||
|
|
@ -58,18 +56,6 @@ func (m *mockConnection) LocalAddr() net.Addr { return m.localAddr }
|
|||
func (m *mockConnection) RemoteAddr() net.Addr { return m.remoteAddr }
|
||||
func (*mockConnection) Close() error { panic("not implemented") }
|
||||
|
||||
func areSessionsRunning() bool {
|
||||
var b bytes.Buffer
|
||||
pprof.Lookup("goroutine").WriteTo(&b, 1)
|
||||
return strings.Contains(b.String(), "quic-go.(*session).run")
|
||||
}
|
||||
|
||||
func areClosedSessionsRunning() bool {
|
||||
var b bytes.Buffer
|
||||
pprof.Lookup("goroutine").WriteTo(&b, 1)
|
||||
return strings.Contains(b.String(), "quic-go.(*closedLocalSession).run")
|
||||
}
|
||||
|
||||
var _ = Describe("Session", func() {
|
||||
var (
|
||||
sess *session
|
||||
|
|
@ -91,17 +77,7 @@ var _ = Describe("Session", func() {
|
|||
}
|
||||
}
|
||||
|
||||
expectReplaceWithClosed := func() {
|
||||
sessionRunner.EXPECT().ReplaceWithClosed(sess.srcConnID, gomock.Any()).Do(func(_ protocol.ConnectionID, s packetHandler) {
|
||||
Expect(s).To(BeAssignableToTypeOf(&closedLocalSession{}))
|
||||
Expect(s.Close()).To(Succeed())
|
||||
Eventually(areClosedSessionsRunning).Should(BeFalse())
|
||||
})
|
||||
}
|
||||
|
||||
BeforeEach(func() {
|
||||
Eventually(areSessionsRunning).Should(BeFalse())
|
||||
|
||||
sessionRunner = NewMockSessionRunner(mockCtrl)
|
||||
mconn = newMockConnection()
|
||||
tokenGenerator, err := handshake.NewTokenGenerator()
|
||||
|
|
@ -131,7 +107,9 @@ var _ = Describe("Session", func() {
|
|||
})
|
||||
|
||||
AfterEach(func() {
|
||||
Eventually(areSessionsRunning).Should(BeFalse())
|
||||
if sess.closedSession != nil {
|
||||
sess.closedSession.destroy()
|
||||
}
|
||||
})
|
||||
|
||||
Context("frame handling", func() {
|
||||
|
|
@ -348,9 +326,7 @@ var _ = Describe("Session", func() {
|
|||
It("handles CONNECTION_CLOSE frames, with a transport error code", func() {
|
||||
testErr := qerr.Error(qerr.StreamLimitError, "foobar")
|
||||
streamManager.EXPECT().CloseWithError(testErr)
|
||||
sessionRunner.EXPECT().ReplaceWithClosed(sess.srcConnID, gomock.Any()).Do(func(_ protocol.ConnectionID, s packetHandler) {
|
||||
Expect(s).To(BeAssignableToTypeOf(&closedRemoteSession{}))
|
||||
})
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
|
||||
go func() {
|
||||
|
|
@ -369,9 +345,7 @@ var _ = Describe("Session", func() {
|
|||
It("handles CONNECTION_CLOSE frames, with an application error code", func() {
|
||||
testErr := qerr.ApplicationError(0x1337, "foobar")
|
||||
streamManager.EXPECT().CloseWithError(testErr)
|
||||
sessionRunner.EXPECT().ReplaceWithClosed(sess.srcConnID, gomock.Any()).Do(func(_ protocol.ConnectionID, s packetHandler) {
|
||||
Expect(s).To(BeAssignableToTypeOf(&closedRemoteSession{}))
|
||||
})
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
|
||||
go func() {
|
||||
|
|
@ -419,7 +393,7 @@ var _ = Describe("Session", func() {
|
|||
|
||||
It("shuts down without error", func() {
|
||||
streamManager.EXPECT().CloseWithError(qerr.Error(qerr.NoError, ""))
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{raw: []byte("connection close")}, nil)
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
|
|
@ -431,7 +405,7 @@ var _ = Describe("Session", func() {
|
|||
|
||||
It("only closes once", func() {
|
||||
streamManager.EXPECT().CloseWithError(qerr.Error(qerr.NoError, ""))
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
|
|
@ -444,7 +418,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()))
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
sess.CloseWithError(0x1337, testErr.Error())
|
||||
|
|
@ -475,7 +449,7 @@ var _ = Describe("Session", func() {
|
|||
|
||||
It("cancels the context when the run loop exists", func() {
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
returned := make(chan struct{})
|
||||
|
|
@ -571,7 +545,7 @@ var _ = Describe("Session", func() {
|
|||
cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
|
||||
sess.run()
|
||||
}()
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
sess.handlePacket(getPacket(&wire.ExtendedHeader{
|
||||
Header: wire.Header{DestConnectionID: sess.srcConnID},
|
||||
PacketNumberLen: protocol.PacketNumberLen1,
|
||||
|
|
@ -596,7 +570,7 @@ var _ = Describe("Session", func() {
|
|||
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.ProtocolViolation))
|
||||
close(done)
|
||||
}()
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
sess.handlePacket(getPacket(&wire.ExtendedHeader{
|
||||
Header: wire.Header{DestConnectionID: sess.srcConnID},
|
||||
PacketNumberLen: protocol.PacketNumberLen1,
|
||||
|
|
@ -616,7 +590,7 @@ var _ = Describe("Session", func() {
|
|||
cryptoSetup.EXPECT().RunHandshake().MaxTimes(1)
|
||||
runErr <- sess.run()
|
||||
}()
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
sess.handlePacket(getPacket(&wire.ExtendedHeader{
|
||||
Header: wire.Header{DestConnectionID: sess.srcConnID},
|
||||
PacketNumberLen: protocol.PacketNumberLen1,
|
||||
|
|
@ -643,7 +617,7 @@ var _ = Describe("Session", func() {
|
|||
Expect(err).To(MatchError("PROTOCOL_VIOLATION: empty packet"))
|
||||
close(done)
|
||||
}()
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
sess.handlePacket(getPacket(&wire.ExtendedHeader{
|
||||
Header: wire.Header{DestConnectionID: sess.srcConnID},
|
||||
PacketNumberLen: protocol.PacketNumberLen1,
|
||||
|
|
@ -844,7 +818,7 @@ var _ = Describe("Session", func() {
|
|||
AfterEach(func() {
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.Close()
|
||||
Eventually(sess.Context().Done()).Should(BeClosed())
|
||||
|
|
@ -946,7 +920,7 @@ var _ = Describe("Session", func() {
|
|||
AfterEach(func() {
|
||||
// make the go routine return
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
Eventually(sess.Context().Done()).Should(BeClosed())
|
||||
|
|
@ -1063,7 +1037,7 @@ var _ = Describe("Session", func() {
|
|||
sess.scheduleSending()
|
||||
Eventually(mconn.written).Should(Receive())
|
||||
// make the go routine return
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
|
|
@ -1097,7 +1071,7 @@ var _ = Describe("Session", func() {
|
|||
Eventually(mconn.written).Should(Receive())
|
||||
// make sure the go routine returns
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.Close()
|
||||
|
|
@ -1121,7 +1095,7 @@ var _ = Describe("Session", func() {
|
|||
Eventually(handshakeCtx.Done()).Should(BeClosed())
|
||||
// make sure the go routine returns
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
|
|
@ -1131,7 +1105,7 @@ var _ = Describe("Session", func() {
|
|||
It("doesn't cancel the HandshakeComplete context when the handshake fails", func() {
|
||||
packer.EXPECT().PackPacket().AnyTimes()
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
go func() {
|
||||
|
|
@ -1165,7 +1139,7 @@ var _ = Describe("Session", func() {
|
|||
Eventually(done).Should(BeClosed())
|
||||
// make sure the go routine returns
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
|
|
@ -1181,7 +1155,7 @@ var _ = Describe("Session", func() {
|
|||
close(done)
|
||||
}()
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
|
|
@ -1199,7 +1173,7 @@ var _ = Describe("Session", func() {
|
|||
close(done)
|
||||
}()
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.CloseWithError(0x1337, testErr.Error())).To(Succeed())
|
||||
|
|
@ -1216,7 +1190,7 @@ var _ = Describe("Session", func() {
|
|||
Expect(err.Error()).To(ContainSubstring("transport parameter"))
|
||||
}()
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.processTransportParameters([]byte("invalid"))
|
||||
|
|
@ -1244,7 +1218,7 @@ var _ = Describe("Session", func() {
|
|||
|
||||
// make the go routine return
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.Close()
|
||||
|
|
@ -1263,7 +1237,7 @@ var _ = Describe("Session", func() {
|
|||
|
||||
AfterEach(func() {
|
||||
// make the go routine return
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
streamManager.EXPECT().CloseWithError(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
|
|
@ -1371,7 +1345,7 @@ var _ = Describe("Session", func() {
|
|||
}()
|
||||
Consistently(sess.Context().Done()).ShouldNot(BeClosed())
|
||||
// make the go routine return
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.Close()
|
||||
Eventually(sess.Context().Done()).Should(BeClosed())
|
||||
|
|
@ -1410,7 +1384,7 @@ var _ = Describe("Session", func() {
|
|||
Consistently(sess.Context().Done()).ShouldNot(BeClosed())
|
||||
// make the go routine return
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.Close()
|
||||
Eventually(sess.Context().Done()).Should(BeClosed())
|
||||
|
|
@ -1512,13 +1486,6 @@ var _ = Describe("Client Session", func() {
|
|||
}
|
||||
}
|
||||
|
||||
expectReplaceWithClosed := func() {
|
||||
sessionRunner.EXPECT().ReplaceWithClosed(sess.srcConnID, gomock.Any()).Do(func(_ protocol.ConnectionID, s packetHandler) {
|
||||
Expect(s.Close()).To(Succeed())
|
||||
Eventually(areClosedSessionsRunning).Should(BeFalse())
|
||||
})
|
||||
}
|
||||
|
||||
BeforeEach(func() {
|
||||
quicConf = populateClientConfig(&Config{}, true)
|
||||
})
|
||||
|
|
@ -1552,6 +1519,12 @@ var _ = Describe("Client Session", func() {
|
|||
sess.cryptoStreamHandler = cryptoSetup
|
||||
})
|
||||
|
||||
AfterEach(func() {
|
||||
if sess.closedSession != nil {
|
||||
sess.closedSession.destroy()
|
||||
}
|
||||
})
|
||||
|
||||
It("changes the connection ID when receiving the first packet from the server", func() {
|
||||
unpacker := NewMockUnpacker(mockCtrl)
|
||||
unpacker.EXPECT().Unpack(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(hdr *wire.Header, _ time.Time, data []byte) (*unpackedPacket, error) {
|
||||
|
|
@ -1581,7 +1554,7 @@ var _ = Describe("Client Session", func() {
|
|||
}, []byte{0}))).To(BeTrue())
|
||||
// make sure the go routine returns
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
cryptoSetup.EXPECT().Close()
|
||||
Expect(sess.Close()).To(Succeed())
|
||||
Eventually(sess.Context().Done()).Should(BeClosed())
|
||||
|
|
@ -1663,7 +1636,7 @@ var _ = Describe("Client Session", func() {
|
|||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("transport parameter"))
|
||||
}()
|
||||
expectReplaceWithClosed()
|
||||
sessionRunner.EXPECT().Retire(gomock.Any())
|
||||
packer.EXPECT().PackConnectionClose(gomock.Any()).Return(&packedPacket{}, nil)
|
||||
cryptoSetup.EXPECT().Close()
|
||||
sess.processTransportParameters([]byte("invalid"))
|
||||
|
|
@ -1766,7 +1739,6 @@ var _ = Describe("Client Session", func() {
|
|||
// Illustrates that an injected Initial with a CONNECTION_CLOSE frame causes
|
||||
// the connection to immediately break down
|
||||
It("fails on Initial-level CONNECTION_CLOSE frame", func() {
|
||||
sessionRunner.EXPECT().ReplaceWithClosed(gomock.Any(), gomock.Any())
|
||||
connCloseFrame := testutils.ComposeConnCloseFrame()
|
||||
initialPacket := testutils.ComposeInitialPacket(sess.destConnID, sess.srcConnID, sess.version, sess.destConnID, []wire.Frame{connCloseFrame})
|
||||
Expect(sess.handlePacketImpl(wrapPacket(initialPacket))).To(BeTrue())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue