gbn+mailbox: force connection status change on FIN

When the client receives+processes a FIN packet, we force the connection
to status ClientStatusSessionNotFound, as in rare occasions the
corresponding server may have time to close the connection before we've
already processed the sent FIN packet by the server. In that case, if we
didn't force a new status, the client would never mark the connection as
status ClientStatusSessionNotFound.

This solves a bug for the testServerReconnect itest, as that test would
sometimes fail due to the FIN packet being processed but without
changing the connection status, leading to a test predicate never being
satisfied.

Author: ViktorTigerstrom

gbn/config.go

@@ -38,6 +38,10 @@ type config struct {
 	// packet.
 	sendToStream sendBytesFunc
 
+	// onFIN is a callback that if set, will be called once a FIN packet has
+	// been received and processed.
+	onFIN func()
+
 	// handshakeTimeout is the time after which the server or client
 	// will abort and restart the handshake if the expected response is
 	// not received from the peer.

gbn/gbn_conn.go

@@ -649,6 +649,10 @@ func (g *GoBackNConn) receivePacketsForever() error { // nolint:gocyclo
 
 			close(g.remoteClosed)
 
+			if g.cfg.onFIN != nil {
+				g.cfg.onFIN()
+			}
+
 			return errTransportClosing
 
 		default:

gbn/options.go

@@ -43,3 +43,11 @@ func WithKeepalivePing(ping, pong time.Duration) Option {
 		conn.pongTime = pong
 	}
 }
+
+// WithOnFIN is used to set the onFIN callback that will be called once a FIN
+// packet has been received and processed.
+func WithOnFIN(fn func()) Option {
+	return func(conn *config) {
+		conn.onFIN = fn
+	}
+}

mailbox/client_conn.go

@@ -157,20 +157,32 @@ func NewClientConn(ctx context.Context, sid [64]byte, serverHost string,
 	}
 
 	c := &ClientConn{
-		transport: transport,
-		gbnOptions: []gbn.Option{
-			gbn.WithTimeout(gbnTimeout),
-			gbn.WithHandshakeTimeout(gbnHandshakeTimeout),
-			gbn.WithKeepalivePing(
-				gbnClientPingTimeout, gbnPongTimeout,
-			),
-		},
+		transport:   transport,
 		status:      ClientStatusNotConnected,
 		onNewStatus: onNewStatus,
 		quit:        make(chan struct{}),
 		cancel:      cancel,
 		log:         logger,
 	}
+
+	c.gbnOptions = []gbn.Option{
+		gbn.WithTimeout(gbnTimeout),
+		gbn.WithHandshakeTimeout(gbnHandshakeTimeout),
+		gbn.WithKeepalivePing(
+			gbnClientPingTimeout, gbnPongTimeout,
+		),
+		gbn.WithOnFIN(func() {
+			// We force the connection to set a new status after
+			// processing a FIN packet, as in rare occasions the
+			// corresponding server may have time to close the
+			// connection before we've already processed the sent
+			// FIN packet by the server. In that case, if we didn't
+			// force a new status, the client would never mark the
+			// connection as status ClientStatusSessionNotFound.
+			c.setStatus(ClientStatusSessionNotFound)
+		}),
+	}
+
 	c.connKit = &connKit{
 		ctx:        ctxc,
 		impl:       c,