fix(dns): use write lock when updating cache entry addresses - #1932
fix(dns): use write lock when updating cache entry addresses#1932magic-peach wants to merge 1 commit into
Conversation
resolve() takes RLock before writing entry.Addresses, which is a read lock and doesn't actually protect against concurrent writers. Two refreshes for the same domain landing at once race on that write - easy to hit under -race with a couple of goroutines resolving the same domain. Switched it to Lock/Unlock like the rest of the mutating paths in this file already do. Added a test that fires several concurrent resolve() calls for one domain and runs clean under -race with the fix, and reliably flags the race without it. Signed-off-by: Akanksha Trehun <akankshatrehun@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
/kind bug
resolve() grabs RLock before doing
entry.Addresses = addrs, which is a read lock and gives no protection against a concurrent writer. If two refreshes for the same domain land at the same time (easy to hit once refreshDns is running against a live queue), they race writing to the same slice field.Switched both to Lock/Unlock, matching how the rest of the mutating paths in this file already handle the cache (RemoveUnwatchDomain, AddDomainInQueue, etc).
Added a test that spins up several concurrent resolve() calls for one domain. Confirmed it flags the race under -race on main and is clean with the fix.
Fixes: N/A, found by reading through the resolver while poking at something else.