Skip to content

Fix port tracking for implicit binds resulting from accept() calls#40287

Draft
FetoiuCatalin wants to merge 4 commits intomasterfrom
user/cfetoiu/accept_porttracking
Draft

Fix port tracking for implicit binds resulting from accept() calls#40287
FetoiuCatalin wants to merge 4 commits intomasterfrom
user/cfetoiu/accept_porttracking

Conversation

@FetoiuCatalin
Copy link
Copy Markdown
Contributor

Summary of the Pull Request

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Copilot AI review requested due to automatic review settings April 23, 2026 17:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Linux-side GnsPortTracker port refresh logic so that tracked port allocations are not deallocated while any active socket is still using the same (protocol, port), including sockets that implicitly “inherit” the local port via accept().

Changes:

  • Adjust OnRefreshAllocatedPorts to consider any active socket using the same protocol+port as a match (not requiring an exact PortAllocation match).
  • Add explanatory comments clarifying why implicit binds (e.g., accepted sockets) must keep allocations alive.

Comment thread src/linux/init/GnsPortTracker.cpp
return p.Port == it->first.Port && p.Protocol == it->first.Protocol;
};

if (std::find_if(Ports.begin(), Ports.end(), matchCondition) == Ports.end())
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new use of std::find_if requires <algorithm> to be included directly in this translation unit (or in a header it includes). Relying on transitive standard-library includes is non-portable and can break builds on other toolchains; add #include <algorithm> near the top of this file.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FetoiuCatalin , please add this include.

return p.Port == it->first.Port && p.Protocol == it->first.Protocol;
};

if (std::find_if(Ports.begin(), Ports.end(), matchCondition) == Ports.end())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FetoiuCatalin , please add this include.

@FetoiuCatalin
Copy link
Copy Markdown
Contributor Author

todo add automated tests for the scenario of opening listen socket, accepting a connection on that socket, closing the listen socket, verify the traffic still flows on the connection socket

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants