Bugfix to terminate the protocol loop on StateError.

The change to introduce worker goroutines can cause the endpoint
to transition to StateError and we should terminate the loop rather
than let the endpoint transition to a CLOSED state as we do
in case the endpoint enters TIME-WAIT/CLOSED. Moving to a closed
state would cause the actual error to not be propagated to
any read() calls etc.

PiperOrigin-RevId: 289923568
This commit is contained in:
Bhasker Hariharan
2020-01-15 13:21:50 -08:00
committed by gVisor bot
parent f874723e64
commit 275ac8ce1d
3 changed files with 41 additions and 24 deletions
+34 -18
View File
@@ -1019,10 +1019,6 @@ func (e *endpoint) handleSegments(fastPath bool) *tcpip.Error {
cont, err := e.handleSegment(s)
if err != nil {
s.decRef()
e.mu.Lock()
e.setEndpointState(StateError)
e.HardError = err
e.mu.Unlock()
return err
}
if !cont {
@@ -1414,30 +1410,50 @@ func (e *endpoint) protocolMainLoop(handshake bool, wakerInitDone chan<- struct{
// Main loop. Handle segments until both send and receive ends of the
// connection have completed.
cleanupOnError := func(err *tcpip.Error) {
e.mu.Lock()
e.workerCleanup = true
if err != nil {
e.resetConnectionLocked(err)
}
// Lock released below.
epilogue()
}
loop:
for e.EndpointState() != StateTimeWait && e.EndpointState() != StateClose && e.EndpointState() != StateError {
e.mu.Unlock()
e.workMu.Unlock()
v, _ := s.Fetch(true)
e.workMu.Lock()
// We need to double check here because the notification
// maybe stale by the time we got around to processing it.
// We need to double check here because the notification maybe
// stale by the time we got around to processing it.
//
// NOTE: since we now hold the workMu the processors cannot
// change the state of the endpoint so it' safe to proceed
// change the state of the endpoint so it's safe to proceed
// after this check.
if e.EndpointState() == StateTimeWait || e.EndpointState() == StateClose || e.EndpointState() == StateError {
e.mu.Lock()
break
}
if err := funcs[v].f(); err != nil {
e.mu.Lock()
e.workerCleanup = true
e.resetConnectionLocked(err)
// Lock released below.
epilogue()
switch e.EndpointState() {
case StateError:
// If the endpoint has already transitioned to an ERROR
// state just pass nil here as any reset that may need
// to be sent etc should already have been done and we
// just want to terminate the loop and cleanup the
// endpoint.
cleanupOnError(nil)
return nil
case StateTimeWait:
fallthrough
case StateClose:
e.mu.Lock()
break loop
default:
if err := funcs[v].f(); err != nil {
cleanupOnError(err)
return nil
}
e.mu.Lock()
}
e.mu.Lock()
}
state := e.EndpointState()
+6
View File
@@ -119,6 +119,12 @@ func (p *processor) handleSegments() {
// direct delivery to ensure low latency and avoid
// scheduler interactions.
if err := ep.handleSegments(true /* fastPath */); err != nil || ep.EndpointState() == StateClose {
// Send any active resets if required.
if err != nil {
ep.mu.Lock()
ep.resetConnectionLocked(err)
ep.mu.Unlock()
}
ep.notifyProtocolGoroutine(notifyTickleWorker)
ep.workMu.Unlock()
continue
+1 -6
View File
@@ -2019,18 +2019,13 @@ func (e *endpoint) Shutdown(flags tcpip.ShutdownFlags) *tcpip.Error {
// If we're fully closed and we have unread data we need to abort
// the connection with a RST.
if (e.shutdownFlags&tcpip.ShutdownWrite) != 0 && rcvBufUsed > 0 {
// Move the socket to error state immediately.
// This is done redundantly because in case of
// save/restore on a Shutdown/Close() the socket
// state needs to indicate the error otherwise
// save file will show the socket in established
// state even though snd/rcv are closed.
e.mu.Unlock()
// Try to send an active reset immediately if the
// work mutex is available.
if e.workMu.TryLock() {
e.mu.Lock()
e.resetConnectionLocked(tcpip.ErrConnectionAborted)
e.notifyProtocolGoroutine(notifyTickleWorker)
e.mu.Unlock()
e.workMu.Unlock()
} else {