[PR #5470] [client] Fix close of closed channel panic in ConnectClient retry loop #28005

Closed
opened 2026-08-05 07:09:31 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/5470

State: closed
Merged: Yes


Describe your changes

When connectWithRetryRuns retries after a connection drop, it passes the same runningChan to a new c.run() call. The first successful connection closes the channel and nils the local parameter, but the caller still holds the original (closed) reference. On retry, close() is called on the already-closed channel, causing a panic.

Fixed by checking if the channel is already closed via non-blocking receive before closing.

Panic trace
panic: close of closed channel [recovered]

goroutine 109 [running]:
github.com/netbirdio/netbird/client/internal.(*ConnectClient).run.func3()
    client/internal/connect.go:334 +0x1e45
github.com/cenkalti/backoff/v4.Operation.withEmptyData.func1()
    github.com/cenkalti/backoff/v4@v4.3.0/retry.go:18 +0x26
github.com/cenkalti/backoff/v4.doRetryNotify[...](0xc000898350, {0x239a1b0, 0xc000a06240}, 0x0, {0x23a3db0, 0xc0012ce060})
    github.com/cenkalti/backoff/v4@v4.3.0/retry.go:88 +0x1ce
github.com/cenkalti/backoff/v4.RetryNotifyWithTimer(0xc00014cfc0, {0x239a1b0, 0xc000a06240}, 0x0, {0x0, 0x0})
    github.com/cenkalti/backoff/v4@v4.3.0/retry.go:61 +0x9e
github.com/cenkalti/backoff/v4.RetryNotify(...)
github.com/cenkalti/backoff/v4.Retry(...)
github.com/netbirdio/netbird/client/internal.(*ConnectClient).run(0xc000f48000, ...)
    client/internal/connect.go:367 +0xf93
github.com/netbirdio/netbird/client/internal.(*ConnectClient).Run(0xc000f48000, 0xc000584460, {0x0, 0x0})
    client/internal/connect.go:75 +0x85
github.com/netbirdio/netbird/client/server.(*Server).connect(0xc0000fa000, ...)
    client/server/server.go:1612 +0x145
github.com/netbirdio/netbird/client/server.(*Server).connectWithRetryRuns.func3()
    client/server/server.go:239 +0x97
github.com/cenkalti/backoff/v4.doRetryNotify[...](...)
    github.com/cenkalti/backoff/v4@v4.3.0/retry.go:88 +0x1ce

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Bug fix, no API or behavior change.

**Original Pull Request:** https://github.com/netbirdio/netbird/pull/5470 **State:** closed **Merged:** Yes --- ## Describe your changes When `connectWithRetryRuns` retries after a connection drop, it passes the same `runningChan` to a new `c.run()` call. The first successful connection closes the channel and nils the local parameter, but the caller still holds the original (closed) reference. On retry, `close()` is called on the already-closed channel, causing a panic. Fixed by checking if the channel is already closed via non-blocking receive before closing. <details> <summary>Panic trace</summary> ``` panic: close of closed channel [recovered] goroutine 109 [running]: github.com/netbirdio/netbird/client/internal.(*ConnectClient).run.func3() client/internal/connect.go:334 +0x1e45 github.com/cenkalti/backoff/v4.Operation.withEmptyData.func1() github.com/cenkalti/backoff/v4@v4.3.0/retry.go:18 +0x26 github.com/cenkalti/backoff/v4.doRetryNotify[...](0xc000898350, {0x239a1b0, 0xc000a06240}, 0x0, {0x23a3db0, 0xc0012ce060}) github.com/cenkalti/backoff/v4@v4.3.0/retry.go:88 +0x1ce github.com/cenkalti/backoff/v4.RetryNotifyWithTimer(0xc00014cfc0, {0x239a1b0, 0xc000a06240}, 0x0, {0x0, 0x0}) github.com/cenkalti/backoff/v4@v4.3.0/retry.go:61 +0x9e github.com/cenkalti/backoff/v4.RetryNotify(...) github.com/cenkalti/backoff/v4.Retry(...) github.com/netbirdio/netbird/client/internal.(*ConnectClient).run(0xc000f48000, ...) client/internal/connect.go:367 +0xf93 github.com/netbirdio/netbird/client/internal.(*ConnectClient).Run(0xc000f48000, 0xc000584460, {0x0, 0x0}) client/internal/connect.go:75 +0x85 github.com/netbirdio/netbird/client/server.(*Server).connect(0xc0000fa000, ...) client/server/server.go:1612 +0x145 github.com/netbirdio/netbird/client/server.(*Server).connectWithRetryRuns.func3() client/server/server.go:239 +0x97 github.com/cenkalti/backoff/v4.doRetryNotify[...](...) github.com/cenkalti/backoff/v4@v4.3.0/retry.go:88 +0x1ce ``` </details> ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Bug fix, no API or behavior change.
saavagebueno added the pull-request label 2026-08-05 07:09:31 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28005