Skip to content

protect yurthub transport client map from races#2510

Open
shiavm006 wants to merge 4 commits into
openyurtio:masterfrom
shiavm006:fix-transport-map-race
Open

protect yurthub transport client map from races#2510
shiavm006 wants to merge 4 commits into
openyurtio:masterfrom
shiavm006:fix-transport-map-race

Conversation

@shiavm006
Copy link
Copy Markdown
Contributor

  • Add sync.RWMutex to transportAndClientManager and fakeTransportManager to guard access to serverToClientset.
  • Build serverToClientset maps completely before publishing them on the manager structs to avoid concurrent writes.
  • Change ListDirectClientset in both managers to return a shallow copy of the internal map, preventing external callers from mutating internal state.
  • Keep the existing Interface API unchanged while ensuring safe concurrent reads and better encapsulation.

@shiavm006 shiavm006 requested a review from a team as a code owner January 29, 2026 04:08
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 77.27273% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.25%. Comparing base (3381599) to head (29cc9f5).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
pkg/yurthub/transport/transport.go 60.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2510      +/-   ##
==========================================
+ Coverage   44.12%   44.25%   +0.12%     
==========================================
  Files         399      399              
  Lines       26564    26590      +26     
==========================================
+ Hits        11722    11768      +46     
+ Misses      13779    13758      -21     
- Partials     1063     1064       +1     
Flag Coverage Δ
unittests 44.25% <77.27%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shiavm006
Copy link
Copy Markdown
Contributor Author

shiavm006 commented Jan 29, 2026

@rambohe-ch @zyjhtangtang there coverage is 0% because there are no tests for the transport package should i add one for it ?
like little bit confused as no prexisting one for this package

@sonarqubecloud
Copy link
Copy Markdown

@shiavm006
Copy link
Copy Markdown
Contributor Author

@rambohe-ch any thoughts ?

@shiavm006
Copy link
Copy Markdown
Contributor Author

@zyjhtangtang would u like to look in to it

@shiavm006
Copy link
Copy Markdown
Contributor Author

@zyjhtangtang could u look into this

@shiavm006
Copy link
Copy Markdown
Contributor Author

@zyjhtangtang any thoughts on this

@shiavm006
Copy link
Copy Markdown
Contributor Author

@zyjhtangtang ??

@stale
Copy link
Copy Markdown

stale Bot commented May 17, 2026

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the wontfix label May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant