[PR #6626] [MERGED] [client] Fix race between WG watcher initial handshake read and endpoint creation #29741

Closed
opened 2026-08-05 08:08:50 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6626
Author: @pappz
Created: 7/1/2026
Status: Merged
Merged: 7/1/2026
Merged by: @pappz

Base: mainHead: fix/wg-watcher-initial-handshake-race


📝 Commits (2)

  • 766becb [client] Fix race between WG watcher initial handshake read and endpoint config
  • c2194f4 [client] Skip WG watcher disconnect callback when context is cancelled

📊 Changes

3 files changed (+43 additions, -26 deletions)

View changed files

📝 client/internal/peer/conn.go (+10 -8)
📝 client/internal/peer/wg_watcher.go (+23 -18)
📝 client/internal/peer/wg_watcher_test.go (+10 -0)

📄 Description

The watcher's initial handshake read ran in a separate goroutine with no ordering guarantee relative to the WireGuard endpoint configuration, so it would sometimes race with the peer being added to the interface. Split enabling into a synchronous PrepareInitialHandshake, called before the endpoint is configured, and an EnableWgWatcher that only runs the monitoring loop, making the baseline read deterministic and keeping it correct for reconnects where the peer's WireGuard entry survives.

Describe your changes

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)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

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)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Improved WireGuard peer monitoring startup so the watcher only begins after capturing the initial handshake baseline, reducing missed or inconsistent connection tracking.
    • Re-enabling the watcher after a stop now uses the same initialization flow for more reliable reconnect handling.
  • Tests
    • Updated WireGuard watcher tests to align with the new startup sequence and assertion style.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6626 **Author:** [@pappz](https://github.com/pappz) **Created:** 7/1/2026 **Status:** ✅ Merged **Merged:** 7/1/2026 **Merged by:** [@pappz](https://github.com/pappz) **Base:** `main` ← **Head:** `fix/wg-watcher-initial-handshake-race` --- ### 📝 Commits (2) - [`766becb`](https://github.com/netbirdio/netbird/commit/766becb2c54c3c0482abd5cc987f285e0df00c80) [client] Fix race between WG watcher initial handshake read and endpoint config - [`c2194f4`](https://github.com/netbirdio/netbird/commit/c2194f4d4439a8b814a26fed50a6a7deb33663dc) [client] Skip WG watcher disconnect callback when context is cancelled ### 📊 Changes **3 files changed** (+43 additions, -26 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/peer/conn.go` (+10 -8) 📝 `client/internal/peer/wg_watcher.go` (+23 -18) 📝 `client/internal/peer/wg_watcher_test.go` (+10 -0) </details> ### 📄 Description The watcher's initial handshake read ran in a separate goroutine with no ordering guarantee relative to the WireGuard endpoint configuration, so it would sometimes race with the peer being added to the interface. Split enabling into a synchronous PrepareInitialHandshake, called before the endpoint is configured, and an EnableWgWatcher that only runs the monitoring loop, making the baseline read deterministic and keeping it correct for reconnects where the peer's WireGuard entry survives. ## Describe your changes ## 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) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > 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) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved WireGuard peer monitoring startup so the watcher only begins after capturing the initial handshake baseline, reducing missed or inconsistent connection tracking. * Re-enabling the watcher after a stop now uses the same initialization flow for more reliable reconnect handling. * **Tests** * Updated WireGuard watcher tests to align with the new startup sequence and assertion style. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:08:50 -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#29741