Files
netbird/relay/server/relay.go
T
Zoltan Papp e392c5899e [relay] Fix race in Relay server shutdown (#7607)
* [relay] Bind listeners before serving to fix the shutdown race

A shutdown signal that arrives while the relay is still starting races
the listener goroutines. Server.Shutdown read the listener's server
field while Listen was writing it, which the race detector reported,
and when Shutdown won it saw a nil server, returned as if nothing was
running, and the listener then bound and served with nothing left to
stop it. Server.Listen never returned and the process hung on exit.

The listener lifecycle is now split into Bind and Serve. Server.Listen
binds every listener under its mutex before spawning the accept loops,
so the fields Shutdown reads are written before the goroutines exist.
A closed flag on the server makes a Listen that runs after Shutdown
return without binding. A bind failure on one listener shuts down the
ones already bound and surfaces the error at once instead of holding
it in a channel until the surviving listener exits.

* [relay] Mark the server closed before shutting down the relay

Shutdown set the closed flag only after the relay had finished closing
peers, so a Listen that started during that window could bind sockets
and start serving on a server that was already going down. The flag is
now set under listenerMux before the relay shutdown, and Accept closes
connections it receives once the relay is closed instead of leaving
them to the client's timeout.

The tests now cover the public Listen bind failure and a QUIC listener
shut down while blocked in Accept, and they pick ports that are free
for both TCP and UDP, reporting a bind error instead of a timeout.

* Fix tests

* [relay] Test the Listen and Shutdown race and the QUIC bind rollback

The existing tests order Listen and Shutdown deterministically, so the
race the fix targets was never exercised. A new test fires both from a
shared start channel across repeated rounds so either side can take the
lock first, and fails on the hang the old code produced.

The rollback test now drives Server.Listen with the real ws and quic
listeners and a UDP blocker, so the ws socket is bound and released when
the quic bind fails. Both tests pass a real TLS config because a nil one
only yields a quic listener in the devcert build.

* [relay] Bound the Shutdown wait in the concurrent Listen and Shutdown test

* [relay] Keep the default WS port for an empty listen address

net.Listen picks a random port for an empty address, while ListenAndServe
used :http or :https. Apply the same default in Bind so an empty address
keeps listening where it did before.

* [relay] Release the WS socket when Serve fails

ServeTLS can return before it takes ownership of the listener, for example
when no certificate is configured, leaving the socket opened in Bind bound
until Shutdown. Close it in Serve on any error other than a server close.

* [relay] Drain the relay outside the listener lock

Shutdown held listenerMux while the relay closed its peers gracefully, so
ListenerProtocols, and with it the healthcheck, blocked for the whole drain.
Mark the server closed and take the listeners under the lock, then drain and
stop them after releasing it. The closed flag still keeps Listen from
registering new listeners.
2026-10-08 11:36:37 +02:00

205 lines
5.3 KiB
Go

package server
import (
"context"
"fmt"
"net/url"
"sync"
"time"
log "github.com/sirupsen/logrus"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/metric"
"github.com/netbirdio/netbird/relay/healthcheck/peerid"
"github.com/netbirdio/netbird/relay/protocol"
"github.com/netbirdio/netbird/relay/server/listener"
//nolint:staticcheck
"github.com/netbirdio/netbird/relay/metrics"
"github.com/netbirdio/netbird/relay/server/store"
)
type Listener interface {
Bind() error
Serve(func(conn listener.Conn)) error
Shutdown(ctx context.Context) error
Protocol() protocol.Protocol
}
type Config struct {
Meter metric.Meter
ExposedAddress string
TLSSupport bool
AuthValidator Validator
instanceURL url.URL
}
func (c *Config) validate() error {
if c.Meter == nil {
c.Meter = otel.Meter("")
}
if c.ExposedAddress == "" {
return fmt.Errorf("exposed address is required")
}
instanceURL, err := getInstanceURL(c.ExposedAddress, c.TLSSupport)
if err != nil {
return fmt.Errorf("invalid url: %v", err)
}
c.instanceURL = *instanceURL
if c.AuthValidator == nil {
return fmt.Errorf("auth validator is required")
}
return nil
}
// Relay represents the relay server
type Relay struct {
metrics *metrics.Metrics
metricsCancel context.CancelFunc
validator Validator
store *store.Store
notifier *store.PeerNotifier
instanceURL url.URL
exposedAddress string
preparedMsg *preparedMsg
closed bool
closeMu sync.RWMutex
}
// NewRelay creates and returns a new Relay instance.
//
// Parameters:
//
// config: A Config struct that holds the configuration needed to initialize the relay server.
// - Meter: A metric.Meter used for emitting metrics. If not set, a default no-op meter will be used.
// - ExposedAddress: The external address clients use to reach this relay. Required.
// - TLSSupport: A boolean indicating if the relay uses TLS. Affects the generated instance URL.
// - AuthValidator: A Validator implementation used to authenticate peers. Required.
//
// Returns:
//
// A pointer to a Relay instance and an error. If initialization is successful, the error will be nil;
// otherwise, it will contain the reason the relay could not be created (e.g., invalid configuration).
func NewRelay(config Config) (*Relay, error) {
if err := config.validate(); err != nil {
return nil, fmt.Errorf("invalid config: %v", err)
}
ctx, metricsCancel := context.WithCancel(context.Background())
m, err := metrics.NewMetrics(ctx, config.Meter)
if err != nil {
metricsCancel()
return nil, fmt.Errorf("creating app metrics: %v", err)
}
r := &Relay{
metrics: m,
metricsCancel: metricsCancel,
validator: config.AuthValidator,
instanceURL: config.instanceURL,
exposedAddress: config.ExposedAddress,
store: store.NewStore(),
notifier: store.NewPeerNotifier(),
}
r.preparedMsg, err = newPreparedMsg(r.instanceURL.String())
if err != nil {
metricsCancel()
return nil, fmt.Errorf("prepare message: %v", err)
}
return r, nil
}
// Accept start to handle a new peer connection
func (r *Relay) Accept(conn listener.Conn) {
acceptTime := time.Now()
r.closeMu.RLock()
defer r.closeMu.RUnlock()
if r.closed {
if err := conn.Close(); err != nil {
log.Debugf("failed to close connection after shutdown, %s: %s", conn.RemoteAddr(), err)
}
return
}
hsCtx, hsCancel := context.WithTimeout(context.Background(), handshakeTimeout)
defer hsCancel()
h := handshake{
conn: conn,
validator: r.validator,
preparedMsg: r.preparedMsg,
}
peerID, err := h.handshakeReceive(hsCtx)
if err != nil {
if peerid.IsHealthCheck(peerID) {
log.Debugf("health check connection from %s", conn.RemoteAddr())
} else {
log.Errorf("failed to handshake: %s", err)
}
if cErr := conn.Close(); cErr != nil {
log.Errorf("failed to close connection, %s: %s", conn.RemoteAddr(), cErr)
}
return
}
peer := NewPeer(r.metrics, *peerID, conn, r.store, r.notifier)
peer.log.Infof("peer connected from: %s", conn.RemoteAddr())
storeTime := time.Now()
if isReconnection := r.store.AddPeer(peer); isReconnection {
r.metrics.RecordPeerReconnection()
}
r.notifier.PeerCameOnline(peer.ID())
transport := conn.Protocol()
r.metrics.RecordPeerStoreTime(time.Since(storeTime))
r.metrics.PeerConnected(peer.String(), transport)
go func() {
peer.Work()
if deleted := r.store.DeletePeer(peer); deleted {
r.notifier.PeerWentOffline(peer.ID())
}
peer.log.Debugf("relay connection closed")
r.metrics.PeerDisconnected(peer.String(), transport)
}()
if err := h.handshakeResponse(hsCtx); err != nil {
log.Errorf("failed to send handshake response, close peer: %s", err)
peer.Close()
}
r.metrics.RecordAuthenticationTime(time.Since(acceptTime))
}
// Shutdown closes the relay server
// It closes the connection with all peers in gracefully and stops accepting new connections.
func (r *Relay) Shutdown(ctx context.Context) {
log.Infof("close connection with all peers")
r.closeMu.Lock()
defer r.closeMu.Unlock()
wg := sync.WaitGroup{}
peers := r.store.Peers()
for _, v := range peers {
wg.Add(1)
go func(p *Peer) {
p.CloseGracefully(ctx)
wg.Done()
}(v.(*Peer))
}
wg.Wait()
r.metricsCancel()
r.closed = true
}
// InstanceURL returns the instance URL of the relay server
func (r *Relay) InstanceURL() url.URL {
return r.instanceURL
}