don't use an idle timeout before the handshake has completed

This simplifies the timeout logic a bit. Before the handshake has
completed, the only timeout is the handshake timeout. After the
handshake has completed, the only timeout is the idle connection timeout.
This commit is contained in:
Marten Seemann
2017-08-29 15:47:56 +07:00
parent 71e82677e1
commit 5152019554
3 changed files with 15 additions and 23 deletions

View File

@@ -102,7 +102,6 @@ type Config struct {
HandshakeTimeout time.Duration HandshakeTimeout time.Duration
// IdleTimeout is the maximum duration that may pass without any incoming network activity. // IdleTimeout is the maximum duration that may pass without any incoming network activity.
// This value only applies after the handshake has completed. // This value only applies after the handshake has completed.
// Before that, the idle timeout is set to half the duration of the HandshakeTimeout.
// If the timeout is exceeded, the connection is closed. // If the timeout is exceeded, the connection is closed.
// If this value is zero, the timeout is set to 30 seconds. // If this value is zero, the timeout is set to 30 seconds.
IdleTimeout time.Duration IdleTimeout time.Duration

View File

@@ -323,12 +323,12 @@ runLoop:
if !s.receivedTooManyUndecrytablePacketsTime.IsZero() && s.receivedTooManyUndecrytablePacketsTime.Add(protocol.PublicResetTimeout).Before(now) && len(s.undecryptablePackets) != 0 { if !s.receivedTooManyUndecrytablePacketsTime.IsZero() && s.receivedTooManyUndecrytablePacketsTime.Add(protocol.PublicResetTimeout).Before(now) && len(s.undecryptablePackets) != 0 {
s.closeLocal(qerr.Error(qerr.DecryptionFailure, "too many undecryptable packets received")) s.closeLocal(qerr.Error(qerr.DecryptionFailure, "too many undecryptable packets received"))
} }
if now.Sub(s.lastNetworkActivityTime) >= s.idleTimeout() {
s.closeLocal(qerr.Error(qerr.NetworkIdleTimeout, "No recent network activity."))
}
if !s.handshakeComplete && now.Sub(s.sessionCreationTime) >= s.config.HandshakeTimeout { if !s.handshakeComplete && now.Sub(s.sessionCreationTime) >= s.config.HandshakeTimeout {
s.closeLocal(qerr.Error(qerr.HandshakeTimeout, "Crypto handshake did not complete in time.")) s.closeLocal(qerr.Error(qerr.HandshakeTimeout, "Crypto handshake did not complete in time."))
} }
if s.handshakeComplete && now.Sub(s.lastNetworkActivityTime) >= s.idleTimeout() {
s.closeLocal(qerr.Error(qerr.NetworkIdleTimeout, "No recent network activity."))
}
s.garbageCollectStreams() s.garbageCollectStreams()
} }
@@ -373,10 +373,7 @@ func (s *session) maybeResetTimer() {
} }
func (s *session) idleTimeout() time.Duration { func (s *session) idleTimeout() time.Duration {
if s.handshakeComplete { return s.connectionParameters.GetIdleConnectionStateLifetime()
return s.connectionParameters.GetIdleConnectionStateLifetime()
}
return s.config.HandshakeTimeout / 2
} }
func (s *session) handlePacketImpl(p *receivedPacket) error { func (s *session) handlePacketImpl(p *receivedPacket) error {

View File

@@ -1507,6 +1507,7 @@ var _ = Describe("Session", func() {
Context("timeouts", func() { Context("timeouts", func() {
It("times out due to no network activity", func(done Done) { It("times out due to no network activity", func(done Done) {
sess.handshakeComplete = true
sess.lastNetworkActivityTime = time.Now().Add(-time.Hour) sess.lastNetworkActivityTime = time.Now().Add(-time.Hour)
err := sess.run() // Would normally not return err := sess.run() // Would normally not return
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout)) Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout))
@@ -1524,27 +1525,22 @@ var _ = Describe("Session", func() {
close(done) close(done)
}) })
It("does not use ICSL before handshake", func(done Done) { It("does not use ICSL before handshake", func() {
defer sess.Close(nil)
sess.lastNetworkActivityTime = time.Now().Add(-time.Minute) sess.lastNetworkActivityTime = time.Now().Add(-time.Minute)
mockCpm = mocks.NewMockConnectionParametersManager(mockCtrl) mockCpm = mocks.NewMockConnectionParametersManager(mockCtrl)
mockCpm.EXPECT().GetIdleConnectionStateLifetime().Return(9999 * time.Second).AnyTimes() mockCpm.EXPECT().GetIdleConnectionStateLifetime().Return(9999 * time.Second).AnyTimes()
mockCpm.EXPECT().TruncateConnectionID().Return(false).AnyTimes() mockCpm.EXPECT().TruncateConnectionID().Return(false).AnyTimes()
sess.connectionParameters = mockCpm sess.connectionParameters = mockCpm
sess.packer.connectionParameters = mockCpm sess.packer.connectionParameters = mockCpm
err := sess.run() // Would normally not return // the handshake timeout is irrelevant here, since it depends on the time the session was created,
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout)) // and not on the last network activity
Expect(mconn.written).To(Receive(ContainSubstring("No recent network activity."))) done := make(chan struct{})
Expect(sess.Context().Done()).To(BeClosed()) go func() {
close(done) _ = sess.run()
}) close(done)
}()
It("times out if the idle period is longer than half the handshake timeout", func(done Done) { Consistently(done).ShouldNot(BeClosed())
sess.lastNetworkActivityTime = time.Now().Add(-protocol.DefaultHandshakeTimeout / 2).Add(-time.Millisecond)
err := sess.run() // Would normally not return
Expect(err.(*qerr.QuicError).ErrorCode).To(Equal(qerr.NetworkIdleTimeout))
Expect(mconn.written).To(Receive(ContainSubstring("No recent network activity.")))
Expect(sess.Context().Done()).To(BeClosed())
close(done)
}) })
It("uses ICSL after handshake", func(done Done) { It("uses ICSL after handshake", func(done Done) {