Fix a race in TCP endpoint teardown and teardown the stack in tcp_test.

Call stack.Close on stacks when we are done with them in tcp_test. This avoids
leaking resources and reduces the test's flakiness when race/gotsan is enabled.
It also provides test coverage for the race also fixed in this change, which
can be reliably triggered with the stack.Close change (and without the other
changes) when race/gotsan is enabled.

The race was possible when calling Abort (via stack.Close) on an endpoint
processing a SYN segment as part of a passive connect.

Updates #1564

PiperOrigin-RevId: 297685432
This commit is contained in:
Ian Gudger
2020-02-27 14:15:44 -08:00
committed by gVisor bot
parent d9ee81183f
commit c6bdc6b05b
4 changed files with 18 additions and 2 deletions
+1
View File
@@ -236,6 +236,7 @@ func (l *listenContext) createConnectingEndpoint(s *segment, iss seqnum.Value, i
n.effectiveNetProtos = []tcpip.NetworkProtocolNumber{s.route.NetProto}
n.rcvBufSize = int(l.rcvWnd)
n.amss = mssForRoute(&n.route)
n.setEndpointState(StateConnecting)
n.maybeEnableTimestamp(rcvdSynOpts)
n.maybeEnableSACKPermitted(rcvdSynOpts)
+1 -1
View File
@@ -577,7 +577,7 @@ func (h *handshake) execute() *tcpip.Error {
case wakerForNotification:
n := h.ep.fetchNotifications()
if n&notifyClose != 0 {
if (n&notifyClose)|(n&notifyAbort) != 0 {
return tcpip.ErrAborted
}
if n&notifyDrain != 0 {
+15 -1
View File
@@ -798,7 +798,21 @@ func (e *endpoint) Abort() {
// If the endpoint disconnected after the check, nothing needs to be
// done, so sending a notification which will potentially be ignored is
// fine.
if e.EndpointState().connected() {
//
// If the endpoint connecting finishes after the check, the endpoint
// is either in a connected state (where we would notifyAbort anyway),
// SYN-RECV (where we would also notifyAbort anyway), or in an error
// state where nothing is required and the notification can be safely
// ignored.
//
// Endpoints where a Close during connecting or SYN-RECV state would be
// problematic are set to state connecting before being registered (and
// thus possible to be Aborted). They are never available in initial
// state.
//
// Endpoints transitioning from initial to connecting state may be
// safely either closed or sent notifyAbort.
if s := e.EndpointState(); s == StateConnecting || s == StateSynRecv || s.connected() {
e.notifyProtocolGoroutine(notifyAbort)
return
}
@@ -204,6 +204,7 @@ func (c *Context) Cleanup() {
if c.EP != nil {
c.EP.Close()
}
c.Stack().Close()
}
// Stack returns a reference to the stack in the Context.