[management] run cancelPeerRoutines in goroutine in sync

[crn4]

Describe your changes

Issue ticket number and link

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

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

• Refactor
  • Improved synchronization error handling to reduce disruption during failures, making peer disconnection cleanup faster and more responsive so overall service reliability and recovery from sync errors is improved.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors peer cleanup in Sync error paths by adding cancelPeerRoutinesWithoutLock(ctx, accountID, peer) and changing two error paths to call it; cancelPeerRoutines now acquires the per-peer lock and delegates to the new unlocked cleanup routine.

Changes

Cohort / File(s)	Summary
Peer cleanup refactor management/internals/shared/grpc/server.go	Adds cancelPeerRoutinesWithoutLock that performs peer-disconnection cleanup without acquiring the per-peer lock; updates two Sync error paths to call the new function; retains cancelPeerRoutines as a locked entrypoint that delegates to the unlocked cleanup.
Manifest / deps manifest_file, go.mod	Minor manifest/dependency file changes (lines changed: +6/-2).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [management] fix ephemeral peers being not removed #5203: Modified Sync error-path peer cleanup in the same source file; related changes to when/how cancelPeerRoutines is invoked.

Suggested reviewers

• pascal-fischer
• mlsmaycon

Poem

🐰 I nibble at locks, then hop away,
A cleaner path for peers today,
Split the tuck, let routines part,
Quiet disconnections, gentle heart,
Hooray for tidy cleanup play! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description follows the required template structure but lacks critical details: 'Describe your changes' section is empty, no issue ticket/link provided, and explanation for why documentation isn't needed is vague ('bug fix' alone).	Fill in the 'Describe your changes' section with details about the peer routine refactoring, provide an issue ticket number/link, and explain specifically why documentation isn't needed for this internal refactoring.
Title check	⚠️ Warning	The title mentions running cancelPeerRoutines in a goroutine, but the actual changes introduce a new function cancelPeerRoutinesWithoutLock and refactor locking behavior—not goroutine execution.	Update the title to accurately reflect the main change, such as 'Extract cancelPeerRoutinesWithoutLock to separate locked/unlocked cleanup paths' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/unlock-peer-in-sync

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This pull request fixes a potential deadlock issue in the Sync function by running the cancelPeerRoutines cleanup method in a goroutine when errors occur during peer synchronization.

Changes:

• Modified two error paths in the Sync method to call cancelPeerRoutines asynchronously (in a goroutine) instead of synchronously
• This prevents deadlock when the calling function holds a peer lock that cancelPeerRoutines also needs to acquire

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud