diff --git a/internal/healthcheck/checker.go b/internal/healthcheck/checker.go index c396ecba..65a47434 100644 --- a/internal/healthcheck/checker.go +++ b/internal/healthcheck/checker.go @@ -8,7 +8,6 @@ import ( "net" "net/netip" "strings" - "sync" "time" "github.com/qdm12/gluetun/internal/healthcheck/dns" @@ -24,7 +23,6 @@ type Checker struct { icmpTargetIPs []netip.Addr smallCheckType string startupOnFail bool - configMutex sync.Mutex icmpNotPermitted *bool @@ -55,8 +53,6 @@ func NewChecker(logger Logger) *Checker { func (c *Checker) SetConfig(tlsDialAddrs []string, icmpTargets []netip.Addr, smallCheckType string, startupOnFail bool, ) { - c.configMutex.Lock() - defer c.configMutex.Unlock() c.tlsDialAddrs = tlsDialAddrs c.icmpTargetIPs = icmpTargets c.smallCheckType = smallCheckType @@ -166,10 +162,8 @@ func (c *Checker) Stop() error { } func (c *Checker) smallPeriodicCheck(ctx context.Context) error { - c.configMutex.Lock() icmpTargetIPs := make([]netip.Addr, len(c.icmpTargetIPs)) copy(icmpTargetIPs, c.icmpTargetIPs) - c.configMutex.Unlock() tryTimeouts := []time.Duration{ 5 * time.Second, 5 * time.Second, diff --git a/internal/vpn/loop.go b/internal/vpn/loop.go index c83ad11a..eab9e397 100644 --- a/internal/vpn/loop.go +++ b/internal/vpn/loop.go @@ -43,6 +43,7 @@ type Loop struct { start <-chan struct{} running chan<- models.LoopStatus userTrigger bool + healthDone <-chan struct{} // Internal constant values backoffTime time.Duration } @@ -68,6 +69,11 @@ func NewLoop(vpnSettings settings.VPN, ipv6Supported bool, vpnInputPorts []uint1 statusManager := loopstate.New(constants.Stopped, start, running, stop, stopped) state := state.New(statusManager, vpnSettings) + // Initialize healthDone channel to a closed channel so that the first time + // we call <-l.healthDone it does not block + healthDone := make(chan struct{}) + close(healthDone) + return &Loop{ statusManager: statusManager, state: state, @@ -95,6 +101,7 @@ func NewLoop(vpnSettings settings.VPN, ipv6Supported bool, vpnInputPorts []uint1 stop: stop, stopped: stopped, userTrigger: true, + healthDone: healthDone, backoffTime: defaultBackoffTime, } } diff --git a/internal/vpn/tunnelup.go b/internal/vpn/tunnelup.go index 5ff5b644..cc0d89ed 100644 --- a/internal/vpn/tunnelup.go +++ b/internal/vpn/tunnelup.go @@ -31,6 +31,7 @@ func (l *Loop) onTunnelUp(ctx, loopCtx context.Context, data tunnelUpData) { } } + <-l.healthDone // make sure the health checker is stopped before restarting it icmpTargetIPs := l.healthSettings.ICMPTargetIPs if len(icmpTargetIPs) == 1 && icmpTargetIPs[0].IsUnspecified() { icmpTargetIPs = []netip.Addr{data.serverIP} @@ -54,7 +55,12 @@ func (l *Loop) onTunnelUp(ctx, loopCtx context.Context, data tunnelUpData) { // Start collecting health errors asynchronously, since // we should not wait for the code below to complete // to start monitoring health and auto-healing. - go l.collectHealthErrors(ctx, loopCtx, healthErrCh) + // We keep track of when this goroutine is done with the healthDone + // channel to avoid a race condition where the health checker is stopped + // after being reconfigured and restarted, instead of before. + healthDone := make(chan struct{}) + l.healthDone = healthDone + go l.collectHealthErrors(ctx, loopCtx, healthDone, healthErrCh) if *l.dnsLooper.GetSettings().ServerEnabled { _, _ = l.dnsLooper.ApplyStatus(ctx, constants.Running) @@ -86,7 +92,10 @@ func (l *Loop) onTunnelUp(ctx, loopCtx context.Context, data tunnelUpData) { } } -func (l *Loop) collectHealthErrors(ctx, loopCtx context.Context, healthErrCh <-chan error) { +func (l *Loop) collectHealthErrors(ctx, loopCtx context.Context, + done chan<- struct{}, healthErrCh <-chan error, +) { + defer close(done) var previousHealthErr error for { select {