[PR #6978] [MERGED] [client] Fix daemon lock order inversion between SetConfig and login #30066

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6978
Author: @lixmal
Created: 7/30/2026
Status: Merged
Merged: 7/31/2026
Merged by: @lixmal

Base: mainHead: fix-setconfig-login-lock-order


📝 Commits (1)

  • 05f713b Fix SetConfig/Login lock order inversion in the daemon

📊 Changes

2 files changed (+61 additions, -6 deletions)

View changed files

client/server/lock_order_test.go (+51 -0)
📝 client/server/server.go (+10 -6)

📄 Description

Describe your changes

SetConfig and the login path acquired the daemon's server mutex and the guarded-config mutex in opposite orders, so two concurrent daemon requests could wedge each other permanently. SetConfig now takes the guarded-config mutex first, matching the login path, which gives the two locks a single order across the daemon.

  • Fix the lock order inversion between SetConfig and login by acquiring the guarded-config lock before the server lock
  • Add a test that fails on the old order

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)

Internal locking fix with no user-visible surface.

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 configuration updates to prevent potential deadlocks during concurrent login and configuration operations.
    • Preserved existing validation and configuration update behavior.
  • Tests

    • Added coverage to verify safe lock ordering during configuration changes.

🔄 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/6978 **Author:** [@lixmal](https://github.com/lixmal) **Created:** 7/30/2026 **Status:** ✅ Merged **Merged:** 7/31/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `main` ← **Head:** `fix-setconfig-login-lock-order` --- ### 📝 Commits (1) - [`05f713b`](https://github.com/netbirdio/netbird/commit/05f713bfe7f28cf934c7222e5b03cf7712400356) Fix SetConfig/Login lock order inversion in the daemon ### 📊 Changes **2 files changed** (+61 additions, -6 deletions) <details> <summary>View changed files</summary> ➕ `client/server/lock_order_test.go` (+51 -0) 📝 `client/server/server.go` (+10 -6) </details> ### 📄 Description ## Describe your changes `SetConfig` and the login path acquired the daemon's server mutex and the guarded-config mutex in opposite orders, so two concurrent daemon requests could wedge each other permanently. `SetConfig` now takes the guarded-config mutex first, matching the login path, which gives the two locks a single order across the daemon. - Fix the lock order inversion between `SetConfig` and login by acquiring the guarded-config lock before the server lock - Add a test that fails on the old order ## 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 - [x] Created tests that fail without the change (if possible) - [x] 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) Internal locking fix with no user-visible surface. ### 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 configuration updates to prevent potential deadlocks during concurrent login and configuration operations. * Preserved existing validation and configuration update behavior. * **Tests** * Added coverage to verify safe lock ordering during configuration changes. <!-- 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:09:58 -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#30066