[PR #5342] [MERGED] Refactor port forward manager cleanup lifecycle #25610

Open
opened 2026-08-05 07:06:14 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5342
Author: @pappz
Created: 2/16/2026
Status: Merged
Merged: 3/4/2026
Merged by: @lixmal

Base: nat-pmp-upnpHead: fix/nat-pmp-upnp-locks


📝 Commits (1)

  • 53a05a4 Refactor port forward manager cleanup lifecycle

📊 Changes

7 files changed (+163 additions, -213 deletions)

View changed files

📝 client/internal/dns/local/local_test.go (+4 -4)
📝 client/internal/engine.go (+11 -3)
📝 client/internal/peer/worker_ice.go (+1 -1)
client/internal/portforward/env.go (+26 -0)
📝 client/internal/portforward/manager.go (+117 -194)
📝 client/internal/portforward/manager_js.go (+3 -10)
📝 client/internal/portforward/manager_test.go (+1 -1)

📄 Description

Replace async goroutine-based cleanup with a synchronous flow where Start runs cleanup inline after renewLoop exits. Use a stopCtx channel so GracefullyStop can pass its deadline-bounded context to Start's cleanup path. When no graceful stop occurs, Start fires cleanup in a background goroutine with a 10s timeout.

Also fix GetMapping double Lock, renewMapping referencing undefined m.mu, cleanup referencing undefined variables, remove statemanager dependency

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)

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/__


🔄 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/5342 **Author:** [@pappz](https://github.com/pappz) **Created:** 2/16/2026 **Status:** ✅ Merged **Merged:** 3/4/2026 **Merged by:** [@lixmal](https://github.com/lixmal) **Base:** `nat-pmp-upnp` ← **Head:** `fix/nat-pmp-upnp-locks` --- ### 📝 Commits (1) - [`53a05a4`](https://github.com/netbirdio/netbird/commit/53a05a42f11e59192c436bf22e86be306ada229f) Refactor port forward manager cleanup lifecycle ### 📊 Changes **7 files changed** (+163 additions, -213 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/dns/local/local_test.go` (+4 -4) 📝 `client/internal/engine.go` (+11 -3) 📝 `client/internal/peer/worker_ice.go` (+1 -1) ➕ `client/internal/portforward/env.go` (+26 -0) 📝 `client/internal/portforward/manager.go` (+117 -194) 📝 `client/internal/portforward/manager_js.go` (+3 -10) 📝 `client/internal/portforward/manager_test.go` (+1 -1) </details> ### 📄 Description Replace async goroutine-based cleanup with a synchronous flow where Start runs cleanup inline after renewLoop exits. Use a stopCtx channel so GracefullyStop can pass its deadline-bounded context to Start's cleanup path. When no graceful stop occurs, Start fires cleanup in a background goroutine with a 10s timeout. Also fix GetMapping double Lock, renewMapping referencing undefined m.mu, cleanup referencing undefined variables, remove statemanager dependency ## 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 - [x] 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 - [ ] 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/__ --- <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 07:06:14 -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#25610