Skip to content

Fix data race between rule-set updates and matching - #4353

Open
solodecode wants to merge 210 commits into
SagerNet:testingfrom
solodecode:agent/fix-ruleset-reload-race
Open

Fix data race between rule-set updates and matching#4353
solodecode wants to merge 210 commits into
SagerNet:testingfrom
solodecode:agent/fix-ruleset-reload-race

Conversation

@solodecode

Copy link
Copy Markdown

LocalRuleSet.reloadRules and RemoteRuleSet.updateOnce replace s.rules under access, while Match, String, ExtractIPSet, and mergeableRule read it without taking the lock. A local reload or remote update can therefore race with active route matching.

Read s.rules through a snapshot taken under RLock, and protect the writes in Cleanup and Close with Lock. Add a regression test that updates local and remote rule-sets while concurrent readers call Match, String, and mergeableRule.

Tested with go test -race ./route/rule.

@solodecode
solodecode marked this pull request as ready for review July 27, 2026 02:12
nekohasekai and others added 23 commits July 27, 2026 17:34
Since b0c6762, every rule inside a referenced rule-set was evaluated
as if merged into the outer rule, which required tracking per-branch
group states and let outer rules and rule-set rules satisfy each
other's grouped conditions in both directions.

Restrict merging to the only designed case: a rule-set containing
exactly one non-inverted default rule is merged into the outer rule as
before. Any other rule-set now matches as an ordinary condition of the
outer rule: it matches when any of its rules matches on its own, and
its rules no longer exchange grouped match state with the outer rule
in either direction. Multiple referenced rule-sets keep OR semantics.

Flat address rule-sets such as generated geosite/geoip sets contain a
single default rule, so their behavior is unchanged. The group-state
set machinery is replaced by a single required/satisfied mask pair.

Also update the route and DNS rule docs.
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.

4 participants