[client] Fix daemon lock order inversion between SetConfig and login - #6978
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesSetConfig lock ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Release artifactsBuilt for PR head
GHCR images (amd64)
This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy. |
riccardomanfrin
left a comment
There was a problem hiding this comment.
guardedConfigMu covers also checkUpdateSettingsDisabled() and loadMDMPolicy() which read from system so login takes lock, SetConfig will take more time.
Test covers only one direction / order (thouth it's the canonical one). might be good to do another with inverted direction.
SetConfig is stale if t.Fatal fires (line 48) but it's terminating anyway so I guess it's fine..
This PR probably needs backport on 0.76.1 ?



Describe your changes
SetConfigand 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.SetConfignow takes the guarded-config mutex first, matching the login path, which gives the two locks a single order across the daemon.SetConfigand login by acquiring the guarded-config lock before the server lockIssue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
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
Tests