mirrors/uquic
1
0
Fork 0
mirror of https://github.com/refraction-networking/uquic.git synced 2025-04-03 20:27:35 +03:00
Code Issues Projects Releases Packages Wiki Activity

remove shutdown method on the Connection (#4249)

Browse source 
There's no need to have a dedicated shutdown method, as the use case
(shutting down an outgoing connection attempt on context cancellation)
can be achieved by using Connection.destroy.
This commit is contained in:
Marten Seemann 2024-01-18 22:06:04 -08:00 • committed by GitHub
parent d3c2020ecd
commit b3eb375bc1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 39 additions and 130 deletions
Download patch file Download diff file Expand all files Collapse all files

2
client.go
View file

@ -232,7 +232,7 @@ func (c *client) dial(ctx context.Context) error {
select {
case <-ctx.Done():
c.conn.shutdown()
c.conn.destroy(nil)
return context.Cause(ctx)
case err := <-errorChan:
return err

2
client_test.go
View file

@ -87,7 +87,7 @@ var _ = Describe("Client", func() {
AfterEach(func() {
if s, ok := cl.conn.(*connection); ok {
s.shutdown()
s.destroy(nil)
}
Eventually(areConnsRunning).Should(BeFalse())
})

2
closed_conn.go
View file

@ -41,7 +41,6 @@ func (c *closedLocalConn) handlePacket(p receivedPacket) {
c.sendPacket(p.remoteAddr, p.info)
}
func (c *closedLocalConn) shutdown() {}
func (c *closedLocalConn) destroy(error) {}
func (c *closedLocalConn) getPerspective() protocol.Perspective { return c.perspective }
@ -59,6 +58,5 @@ func newClosedRemoteConn(pers protocol.Perspective) packetHandler {
}
func (s *closedRemoteConn) handlePacket(receivedPacket) {}
func (s *closedRemoteConn) shutdown() {}
func (s *closedRemoteConn) destroy(error) {}
func (s *closedRemoteConn) getPerspective() protocol.Perspective { return s.perspective }

2
closed_conn_test.go
View file

@ -14,8 +14,6 @@ var _ = Describe("Closed local connection", func() {
It("tells its perspective", func() {
conn := newClosedLocalConn(nil, protocol.PerspectiveClient, utils.DefaultLogger)
Expect(conn.getPerspective()).To(Equal(protocol.PerspectiveClient))
// stop the connection
conn.shutdown()
})
It("repeats the packet containing the CONNECTION_CLOSE frame", func() {

7
connection.go
View file

@ -1572,13 +1572,6 @@ func (s *connection) closeRemote(e error) {
})
}
// Close the connection. It sends a NO_ERROR application error.
// It waits until the run loop has stopped before returning
func (s *connection) shutdown() {
s.closeLocal(nil)
<-s.ctx.Done()
}
func (s *connection) CloseWithError(code ApplicationErrorCode, desc string) error {
s.closeLocal(&qerr.ApplicationError{
ErrorCode: code,

59
connection_test.go
View file

@ -465,7 +465,7 @@ var _ = Describe("Connection", func() {
}),
tracer.EXPECT().Close(),
)
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(areConnsRunning).Should(BeFalse())
Expect(conn.Context().Done()).To(BeClosed())
})
@ -479,8 +479,8 @@ var _ = Describe("Connection", func() {
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.shutdown()
conn.CloseWithError(0, "")
conn.CloseWithError(0, "")
Eventually(areConnsRunning).Should(BeFalse())
Expect(conn.Context().Done()).To(BeClosed())
})
@ -551,29 +551,6 @@ var _ = Describe("Connection", func() {
}
})
It("cancels the context when the run loop exists", func() {
runConn()
streamManager.EXPECT().CloseWithError(gomock.Any())
expectReplaceWithClosed()
cryptoSetup.EXPECT().Close()
packer.EXPECT().PackApplicationClose(gomock.Any(), gomock.Any(), conn.version).Return(&coalescedPacket{buffer: getPacketBuffer()}, nil)
returned := make(chan struct{})
go func() {
defer GinkgoRecover()
ctx := conn.Context()
<-ctx.Done()
Expect(ctx.Err()).To(MatchError(context.Canceled))
close(returned)
}()
Consistently(returned).ShouldNot(BeClosed())
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
Eventually(returned).Should(BeClosed())
Expect(context.Cause(conn.Context())).To(MatchError(context.Canceled))
})
It("doesn't send any more packets after receiving a CONNECTION_CLOSE", func() {
unpacker := NewMockUnpacker(mockCtrl)
conn.handshakeConfirmed = true
@ -964,7 +941,7 @@ var _ = Describe("Connection", func() {
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -1219,7 +1196,7 @@ var _ = Describe("Connection", func() {
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
sender.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
Eventually(connDone).Should(BeClosed())
})
@ -1422,7 +1399,7 @@ var _ = Describe("Connection", func() {
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
sender.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -1811,7 +1788,7 @@ var _ = Describe("Connection", func() {
sender.EXPECT().Close()
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -1937,7 +1914,7 @@ var _ = Describe("Connection", func() {
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -2059,7 +2036,7 @@ var _ = Describe("Connection", func() {
cryptoSetup.EXPECT().Close()
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -2073,13 +2050,11 @@ var _ = Describe("Connection", func() {
close(done)
}()
streamManager.EXPECT().CloseWithError(gomock.Any())
expectReplaceWithClosed()
packer.EXPECT().PackApplicationClose(gomock.Any(), gomock.Any(), conn.version).Return(&coalescedPacket{buffer: getPacketBuffer()}, nil)
cryptoSetup.EXPECT().Close()
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
connRunner.EXPECT().Remove(gomock.Any()).AnyTimes()
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.destroy(nil)
Eventually(done).Should(BeClosed())
Expect(context.Cause(conn.Context())).To(MatchError(context.Canceled))
})
@ -2165,7 +2140,7 @@ var _ = Describe("Connection", func() {
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -2316,7 +2291,7 @@ var _ = Describe("Connection", func() {
expectReplaceWithClosed()
cryptoSetup.EXPECT().Close()
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -2403,7 +2378,7 @@ var _ = Describe("Connection", func() {
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any())
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
})
@ -2623,7 +2598,7 @@ var _ = Describe("Client Connection", func() {
mconn.EXPECT().Write(gomock.Any(), gomock.Any(), gomock.Any()).MaxTimes(1)
tracer.EXPECT().ClosedConnection(gomock.Any())
tracer.EXPECT().Close()
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
time.Sleep(200 * time.Millisecond)
})
@ -2930,7 +2905,7 @@ var _ = Describe("Client Connection", func() {
}
AfterEach(func() {
conn.shutdown()
conn.CloseWithError(0, "")
Eventually(conn.Context().Done()).Should(BeClosed())
Eventually(errChan).Should(BeClosed())
})
@ -2974,7 +2949,7 @@ var _ = Describe("Client Connection", func() {
Eventually(processed).Should(BeClosed())
// close first
expectClose(true, false)
conn.shutdown()
conn.CloseWithError(0, "")
// then check. Avoids race condition when accessing idleTimeout
Expect(conn.idleTimeout).To(Equal(18 * time.Second))
})

20
integrationtests/self/handshake_test.go
View file

@ -82,6 +82,26 @@ var _ = Describe("Handshake tests", func() {
}()
}
It("returns the context cancellation error on timeouts", func() {
ctx, cancel := context.WithTimeout(context.Background(), scaleDuration(20*time.Millisecond))
defer cancel()
errChan := make(chan error, 1)
go func() {
_, err := quic.DialAddr(
ctx,
"localhost:1234", // nobody is listening on this port, but we're going to cancel this dial anyway
getTLSClientConfig(),
getQuicConfig(nil),
)
errChan <- err
}()
var err error
Eventually(errChan).Should(Receive(&err))
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(context.DeadlineExceeded))
})
It("returns the cancellation reason when a dial is canceled", func() {
ctx, cancel := context.WithCancelCause(context.Background())
errChan := make(chan error, 1)

36
mock_packet_handler_test.go
View file

@ -147,39 +147,3 @@ func (c *PacketHandlerhandlePacketCall) DoAndReturn(f func(receivedPacket)) *Pac
c.Call = c.Call.DoAndReturn(f)
return c
}
// shutdown mocks base method.
func (m *MockPacketHandler) shutdown() {
m.ctrl.T.Helper()
m.ctrl.Call(m, "shutdown")
}
// shutdown indicates an expected call of shutdown.
func (mr *MockPacketHandlerMockRecorder) shutdown() *PacketHandlershutdownCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "shutdown", reflect.TypeOf((*MockPacketHandler)(nil).shutdown))
return &PacketHandlershutdownCall{Call: call}
}
// PacketHandlershutdownCall wrap *gomock.Call
type PacketHandlershutdownCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *PacketHandlershutdownCall) Return() *PacketHandlershutdownCall {
c.Call = c.Call.Return()
return c
}
// Do rewrite *gomock.Call.Do
func (c *PacketHandlershutdownCall) Do(f func()) *PacketHandlershutdownCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *PacketHandlershutdownCall) DoAndReturn(f func()) *PacketHandlershutdownCall {
c.Call = c.Call.DoAndReturn(f)
return c
}

36
mock_quic_conn_test.go
View file

@ -841,39 +841,3 @@ func (c *QUICConnrunCall) DoAndReturn(f func() error) *QUICConnrunCall {
c.Call = c.Call.DoAndReturn(f)
return c
}
// shutdown mocks base method.
func (m *MockQUICConn) shutdown() {
m.ctrl.T.Helper()
m.ctrl.Call(m, "shutdown")
}
// shutdown indicates an expected call of shutdown.
func (mr *MockQUICConnMockRecorder) shutdown() *QUICConnshutdownCall {
mr.mock.ctrl.T.Helper()
call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "shutdown", reflect.TypeOf((*MockQUICConn)(nil).shutdown))
return &QUICConnshutdownCall{Call: call}
}
// QUICConnshutdownCall wrap *gomock.Call
type QUICConnshutdownCall struct {
*gomock.Call
}
// Return rewrite *gomock.Call.Return
func (c *QUICConnshutdownCall) Return() *QUICConnshutdownCall {
c.Call = c.Call.Return()
return c
}
// Do rewrite *gomock.Call.Do
func (c *QUICConnshutdownCall) Do(f func()) *QUICConnshutdownCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *QUICConnshutdownCall) DoAndReturn(f func()) *QUICConnshutdownCall {
c.Call = c.Call.DoAndReturn(f)
return c
}

1
packet_handler_map.go
View file

@ -191,7 +191,6 @@ func (h *packetHandlerMap) ReplaceWithClosed(ids []protocol.ConnectionID, pers p
time.AfterFunc(h.deleteRetiredConnsAfter, func() {
h.mutex.Lock()
handler.shutdown()
for _, id := range ids {
delete(h.handlers, id)
}

2
server.go
View file

@ -24,7 +24,6 @@ var ErrServerClosed = errors.New("quic: server closed")
// packetHandler handles packets
type packetHandler interface {
handlePacket(receivedPacket)
shutdown()
destroy(error)
getPerspective() protocol.Perspective
}
@ -45,7 +44,6 @@ type quicConn interface {
getPerspective() protocol.Perspective
run() error
destroy(error)
shutdown()
}
type zeroRTTQueue struct {