[PR #6807] [client] Scope conntrack cancellation per run #29037

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6807
Author: @CoderSufiyan
Created: 7/16/2026
Status: 🔄 Open

Base: mainHead: fix/issue-5887-conntrack-run


📝 Commits (2)

  • f8a9cb0 [client] Scope conntrack cancellation per run
  • b5e325a [client] Report unexpected reconnect dials safely

📊 Changes

2 files changed (+146 additions, -80 deletions)

View changed files

📝 client/internal/netflow/conntrack/conntrack.go (+78 -73)
📝 client/internal/netflow/conntrack/conntrack_test.go (+68 -7)

📄 Description

Describe your changes

Replace the conntrack listener’s shared stop channel and global connection state with a per-run context and listener owner.

A stopped listener can remain inside reconnect dial or listen after Stop returns. Previously, a following Start could consume the shared stop signal and set started again, allowing the stale reconnect to replace or close the new listener. Reconnect publication and cleanup now verify the originating run identity, so stale work can only close its own candidate connection.

The regression test reproduces a stopped run completing its reconnect dial after a replacement run has started and verifies that the replacement remains current and open.

Fixes #5887

Stack

  • This PR is independent

Checklist

  • I have performed a self-review of my own code
  • I have tested the changes locally
  • I have added or updated tests where applicable
  • I have checked for breaking changes

Documentation

  • Documentation is not needed

Validation

  • Linux amd64 test-binary compilation for ./client/internal/netflow/conntrack
  • GOOS=linux GOARCH=amd64 go vet ./client/internal/netflow/conntrack
  • git diff --check

The Linux-only tests could not be executed on the local macOS host.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes

    • Improved connection tracking lifecycle with per-run cancellation to ensure listeners stop cleanly.
    • Refined reconnection flow to avoid stale reconnect attempts affecting newly started runs.
    • Updated Stop/Close behavior so in-flight reconnects are promptly canceled and associated connections are cleaned up.
  • Tests

    • Enhanced conntrack reconnection/stop concurrency coverage to assert run lifecycle changes during races.
    • Added a race-focused test for stop/restart while a reconnect dial is in flight, verifying the stale connection is closed and the active run remains correct.

🔄 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/6807 **Author:** [@CoderSufiyan](https://github.com/CoderSufiyan) **Created:** 7/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/issue-5887-conntrack-run` --- ### 📝 Commits (2) - [`f8a9cb0`](https://github.com/netbirdio/netbird/commit/f8a9cb0ba4a37adcd58c388016e47d31212e1378) [client] Scope conntrack cancellation per run - [`b5e325a`](https://github.com/netbirdio/netbird/commit/b5e325abb8d2c9afb3bb0e6cbcddb31d0b4142ec) [client] Report unexpected reconnect dials safely ### 📊 Changes **2 files changed** (+146 additions, -80 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/netflow/conntrack/conntrack.go` (+78 -73) 📝 `client/internal/netflow/conntrack/conntrack_test.go` (+68 -7) </details> ### 📄 Description ## Describe your changes Replace the conntrack listener’s shared stop channel and global connection state with a per-run context and listener owner. A stopped listener can remain inside reconnect dial or listen after `Stop` returns. Previously, a following `Start` could consume the shared stop signal and set `started` again, allowing the stale reconnect to replace or close the new listener. Reconnect publication and cleanup now verify the originating run identity, so stale work can only close its own candidate connection. The regression test reproduces a stopped run completing its reconnect dial after a replacement run has started and verifies that the replacement remains current and open. ## Issue ticket number and link Fixes #5887 ## Stack - [x] This PR is independent ## Checklist - [x] I have performed a self-review of my own code - [x] I have tested the changes locally - [x] I have added or updated tests where applicable - [x] I have checked for breaking changes ## Documentation - [x] Documentation is **not needed** ## Validation - Linux amd64 test-binary compilation for `./client/internal/netflow/conntrack` - `GOOS=linux GOARCH=amd64 go vet ./client/internal/netflow/conntrack` - `git diff --check` The Linux-only tests could not be executed on the local macOS host. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6807"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1786811331&installation_id=146802194&pr_number=6807&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6807&signature=56a5f957f51a0a85cf5a0aea85b42d5e10baf873f1d224922c289ec274d1498f"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved connection tracking lifecycle with per-run cancellation to ensure listeners stop cleanly. * Refined reconnection flow to avoid stale reconnect attempts affecting newly started runs. * Updated Stop/Close behavior so in-flight reconnects are promptly canceled and associated connections are cleaned up. * **Tests** * Enhanced conntrack reconnection/stop concurrency coverage to assert run lifecycle changes during races. * Added a race-focused test for stop/restart while a reconnect dial is in flight, verifying the stale connection is closed and the active run remains correct. <!-- 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:07:21 -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#29037